diff options
4 files changed, 256 insertions, 3 deletions
diff --git a/Emby.Server.Implementations/Images/CollectionFolderImageProvider.cs b/Emby.Server.Implementations/Images/CollectionFolderImageProvider.cs index 7cae2a671b..1d3cc3d383 100644 --- a/Emby.Server.Implementations/Images/CollectionFolderImageProvider.cs +++ b/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; diff --git a/Jellyfin.Server.Implementations/Item/BaseItemRepository.ByName.cs b/Jellyfin.Server.Implementations/Item/BaseItemRepository.ByName.cs index 4199f83c21..4e5d561f0a 100644 --- a/Jellyfin.Server.Implementations/Item/BaseItemRepository.ByName.cs +++ b/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, diff --git a/tests/Jellyfin.Server.Implementations.Tests/Images/CollectionFolderImageProviderTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Images/CollectionFolderImageProviderTests.cs new file mode 100644 index 0000000000..b88296a06a --- /dev/null +++ b/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); + } +} diff --git a/tests/Jellyfin.Server.Implementations.Tests/Item/BaseItemRepositoryArtistLibraryScopeTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Item/BaseItemRepositoryArtistLibraryScopeTests.cs new file mode 100644 index 0000000000..1f8d9820d6 --- /dev/null +++ b/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(); + } +} |