Backport pull request #18148 from jellyfin/release-12.z

Scope the music library cover image to that library's artists

Original-merge: c60c5e0516

Merged-by: crobibero <cody@robibe.ro>

Backported-by: Cody Robibero <cody@robibe.ro>

Emby.Server.Implementations/Images/CollectionFolderImageProvider.cs

@@ -5,6 +5,7 @@
 using System;
 using System.Collections.Generic;
 using System.IO;
+using System.Linq;
 using Jellyfin.Api.Extensions;
 using Jellyfin.Data.Enums;
 using Jellyfin.Database.Implementations.Enums;
@@ -12,6 +13,7 @@ using MediaBrowser.Common.Configuration;
 using MediaBrowser.Controller.Drawing;
 using MediaBrowser.Controller.Dto;
 using MediaBrowser.Controller.Entities;
+using MediaBrowser.Controller.Library;
 using MediaBrowser.Controller.Providers;
 using MediaBrowser.Model.Entities;
 using MediaBrowser.Model.IO;
@@ -20,8 +22,11 @@ namespace Emby.Server.Implementations.Images
 {
     public class CollectionFolderImageProvider : BaseDynamicImageProvider<CollectionFolder>
     {
-        public CollectionFolderImageProvider(IFileSystem fileSystem, IProviderManager providerManager, IApplicationPaths applicationPaths, IImageProcessor imageProcessor) : base(fileSystem, providerManager, applicationPaths, imageProcessor)
+        private readonly ILibraryManager _libraryManager;
+
+        public CollectionFolderImageProvider(IFileSystem fileSystem, IProviderManager providerManager, IApplicationPaths applicationPaths, IImageProcessor imageProcessor, ILibraryManager libraryManager) : base(fileSystem, providerManager, applicationPaths, imageProcessor)
         {
+            _libraryManager = libraryManager;
         }
 
         protected override IReadOnlyList<BaseItem> GetItemsWithImages(BaseItem item)
@@ -33,8 +38,11 @@ namespace Emby.Server.Implementations.Images
 
             if (viewType == CollectionType.music)
             {
-                // Music albums usually don't have dedicated backdrops, so use artist instead
-                includeItemTypes = [BaseItemKind.MusicArtist];
+                // Music albums usually don't have dedicated backdrops, so use artist instead.
+                // Artists carry no library of their own, so an item query for them is not
+                // restricted to this library and would collage the artists of every music
+                // library. Resolve them through the tracks that credit them instead.
+                return GetArtistsWithImages(view);
             }
 
             return view.GetItemList(new InternalItemsQuery
@@ -49,6 +57,19 @@ namespace Emby.Server.Implementations.Images
             });
         }
 
+        private IReadOnlyList<BaseItem> GetArtistsWithImages(CollectionFolder view)
+        {
+            return _libraryManager.GetAllArtists(new InternalItemsQuery
+            {
+                AncestorIds = [view.Id],
+                DtoOptions = new DtoOptions(false),
+                EnableTotalRecordCount = false,
+                ImageTypes = [ImageType.Primary],
+                Limit = 8,
+                OrderBy = [(ItemSortBy.Random, SortOrder.Ascending)]
+            }).Items.Select(i => i.Item).ToArray();
+        }
+
         protected override bool Supports(BaseItem item)
         {
             return item is CollectionFolder;

Jellyfin.Server.Implementations/Item/BaseItemRepository.ByName.cs

@@ -212,6 +212,7 @@ public sealed partial class BaseItemRepository
             IsFavoriteOrLiked = filter.IsFavoriteOrLiked,
             IsLiked = filter.IsLiked,
             IsLocked = filter.IsLocked,
+            ImageTypes = filter.ImageTypes,
             NameLessThan = filter.NameLessThan,
             NameStartsWith = filter.NameStartsWith,
             NameStartsWithOrGreater = filter.NameStartsWithOrGreater,

tests/Jellyfin.Server.Implementations.Tests/Images/CollectionFolderImageProviderTests.cs

@@ -0,0 +1,74 @@
+using System;
+using System.Collections.Generic;
+using Emby.Server.Implementations.Images;
+using Jellyfin.Data.Enums;
+using MediaBrowser.Common.Configuration;
+using MediaBrowser.Controller.Drawing;
+using MediaBrowser.Controller.Entities;
+using MediaBrowser.Controller.Entities.Audio;
+using MediaBrowser.Controller.Library;
+using MediaBrowser.Controller.Providers;
+using MediaBrowser.Model.Dto;
+using MediaBrowser.Model.Entities;
+using MediaBrowser.Model.IO;
+using MediaBrowser.Model.Querying;
+using Moq;
+using Xunit;
+
+namespace Jellyfin.Server.Implementations.Tests.Images;
+
+/// <summary>
+/// A music library is collaged from its artists' backdrops. Artists are by-name items with no
+/// library of their own, so they have to be asked for through the by-name listing, which reaches
+/// them through the tracks that credit them; an item query for them ignores the library scope and
+/// hands back the artists of every music library.
+/// </summary>
+public sealed class CollectionFolderImageProviderTests
+{
+    [Fact]
+    public void GetItemsWithImages_MusicLibrary_AsksForTheArtistsOfThatLibraryOnly()
+    {
+        var view = new CollectionFolder { Id = Guid.NewGuid(), CollectionType = CollectionType.music };
+        var artist = new MusicArtist { Id = Guid.NewGuid(), Name = "Artist" };
+
+        InternalItemsQuery? query = null;
+        var libraryManager = new Mock<ILibraryManager>();
+        libraryManager
+            .Setup(l => l.GetAllArtists(It.IsAny<InternalItemsQuery>()))
+            .Callback<InternalItemsQuery>(q => query = q)
+            .Returns(new QueryResult<(BaseItem Item, ItemCounts ItemCounts)>([(artist, new ItemCounts())]));
+
+        var items = CreateProvider(libraryManager.Object).GetItems(view);
+
+        Assert.Equal([artist], items);
+        Assert.NotNull(query);
+        Assert.Equal([view.Id], query.AncestorIds);
+        Assert.Equal([ImageType.Primary], query.ImageTypes);
+        Assert.Equal(8, query.Limit);
+    }
+
+    private static TestableCollectionFolderImageProvider CreateProvider(ILibraryManager libraryManager)
+    {
+        return new TestableCollectionFolderImageProvider(
+            Mock.Of<IFileSystem>(),
+            Mock.Of<IProviderManager>(),
+            Mock.Of<IApplicationPaths>(),
+            Mock.Of<IImageProcessor>(),
+            libraryManager);
+    }
+
+    private sealed class TestableCollectionFolderImageProvider : CollectionFolderImageProvider
+    {
+        public TestableCollectionFolderImageProvider(
+            IFileSystem fileSystem,
+            IProviderManager providerManager,
+            IApplicationPaths applicationPaths,
+            IImageProcessor imageProcessor,
+            ILibraryManager libraryManager)
+            : base(fileSystem, providerManager, applicationPaths, imageProcessor, libraryManager)
+        {
+        }
+
+        public IReadOnlyList<BaseItem> GetItems(BaseItem item) => GetItemsWithImages(item);
+    }
+}

tests/Jellyfin.Server.Implementations.Tests/Item/BaseItemRepositoryArtistLibraryScopeTests.cs

@@ -0,0 +1,157 @@
+using System;
+using System.Linq;
+using Emby.Server.Implementations.Data;
+using Jellyfin.Database.Implementations.Entities;
+using Jellyfin.Server.Implementations.Item;
+using MediaBrowser.Controller.Dto;
+using MediaBrowser.Controller.Entities;
+using MediaBrowser.Model.Entities;
+using Xunit;
+using BaseItemKind = Jellyfin.Data.Enums.BaseItemKind;
+
+namespace Jellyfin.Server.Implementations.Tests.Item;
+
+/// <summary>
+/// Artists are by-name items: they live outside any library and carry no TopParentId, so a plain
+/// item query for them is exempt from the library filter and spans every music library. Only the
+/// by-name listings, which reach the artist through the tracks that credit it, can be scoped to
+/// one library.
+/// </summary>
+public sealed class BaseItemRepositoryArtistLibraryScopeTests : SqliteDbTestFixture
+{
+    private static readonly Guid _firstLibrary = Guid.Parse("11111111-0000-0000-0000-000000000001");
+    private static readonly Guid _secondLibrary = Guid.Parse("22222222-0000-0000-0000-000000000001");
+
+    private readonly BaseItemRepository _repository;
+    private readonly ItemTypeLookup _itemTypeLookup;
+
+    public BaseItemRepositoryArtistLibraryScopeTests()
+    {
+        _itemTypeLookup = new ItemTypeLookup();
+        _repository = CreateBaseItemRepository(_itemTypeLookup);
+
+        Seed("First Artist", "first artist", _firstLibrary, hasImage: true);
+        Seed("Second Artist", "second artist", _secondLibrary, hasImage: true);
+    }
+
+    [Fact]
+    public void GetItemList_MusicArtistsScopedToOneLibrary_ReturnsEveryLibrarysArtists()
+    {
+        // The shape the library cover image used to be built from. By-name types are exempt from
+        // the TopParentId filter, so the scope is silently dropped.
+        var result = _repository.GetItemList(new InternalItemsQuery
+        {
+            DtoOptions = new DtoOptions(false),
+            IncludeItemTypes = [BaseItemKind.MusicArtist],
+            TopParentIds = [_firstLibrary]
+        });
+
+        Assert.Equal(["First Artist", "Second Artist"], result.Select(i => i.Name).OrderBy(n => n));
+    }
+
+    [Fact]
+    public void GetAllArtists_ScopedToOneLibrary_ReturnsOnlyThatLibrarysArtists()
+    {
+        var result = _repository.GetAllArtists(new InternalItemsQuery
+        {
+            DtoOptions = new DtoOptions(false),
+            TopParentIds = [_firstLibrary]
+        });
+
+        var (artist, _) = Assert.Single(result.Items);
+        Assert.Equal("First Artist", artist.Name);
+    }
+
+    [Fact]
+    public void GetAllArtists_ImageTypes_DropsArtistsWithoutThatImage()
+    {
+        // The collage has nothing to draw with an artist that has no image, so the listing has to
+        // honour the image filter the caller asked for.
+        Seed("Third Artist", "third artist", _firstLibrary, hasImage: false);
+
+        var result = _repository.GetAllArtists(new InternalItemsQuery
+        {
+            DtoOptions = new DtoOptions(false),
+            ImageTypes = [ImageType.Primary],
+            TopParentIds = [_firstLibrary]
+        });
+
+        var (artist, _) = Assert.Single(result.Items);
+        Assert.Equal("First Artist", artist.Name);
+    }
+
+    /// <summary>
+    /// Seeds one by-name artist row and a track in the given library crediting it.
+    /// </summary>
+    /// <param name="name">The artist name.</param>
+    /// <param name="cleanName">The cleaned artist name, which is what links the two rows.</param>
+    /// <param name="topParentId">The library the track belongs to.</param>
+    /// <param name="hasImage">Whether the artist row carries a primary image.</param>
+    private void Seed(string name, string cleanName, Guid topParentId, bool hasImage)
+    {
+        using var ctx = CreateDbContext();
+
+        var artistId = Guid.NewGuid();
+        var artist = new BaseItemEntity
+        {
+            Id = artistId,
+            Type = _itemTypeLookup.BaseItemKindNames[BaseItemKind.MusicArtist],
+            Name = name,
+            CleanName = cleanName,
+            PresentationUniqueKey = artistId.ToString("N"),
+            IsFolder = true,
+            IsVirtualItem = false
+        };
+
+        if (hasImage)
+        {
+            artist.Images =
+            [
+                new BaseItemImageInfo
+                {
+                    Id = Guid.NewGuid(),
+                    ItemId = artistId,
+                    Item = artist,
+                    ImageType = ImageInfoImageType.Primary,
+                    Path = $"/metadata/artists/{cleanName}/folder.jpg"
+                }
+            ];
+        }
+
+        ctx.BaseItems.Add(artist);
+
+        var trackId = Guid.NewGuid();
+        var track = new BaseItemEntity
+        {
+            Id = trackId,
+            Type = _itemTypeLookup.BaseItemKindNames[BaseItemKind.Audio],
+            Name = $"{name} - Track",
+            CleanName = $"{cleanName} - track",
+            PresentationUniqueKey = trackId.ToString("N"),
+            MediaType = "Audio",
+            TopParentId = topParentId,
+            IsFolder = false,
+            IsVirtualItem = false
+        };
+        ctx.BaseItems.Add(track);
+
+        var itemValue = new ItemValue
+        {
+            ItemValueId = Guid.NewGuid(),
+            Type = ItemValueType.AlbumArtist,
+            Value = name,
+            CleanValue = cleanName
+        };
+
+        ctx.ItemValues.Add(itemValue);
+        ctx.ItemValuesMap.Add(new ItemValueMap
+        {
+            ItemId = trackId,
+            ItemValueId = itemValue.ItemValueId,
+            Item = track,
+            ItemValue = itemValue
+        });
+
+        ctx.SaveChanges();
+    }
+}