From 4adaf7f146ee895d714ae87af6858101bb10e505 Mon Sep 17 00:00:00 2001 From: vdatanet Date: Wed, 5 Aug 2026 11:34:33 +0200 Subject: Fix by-name endpoints reporting TotalRecordCount=0 next to a populated Items array `GetItemValues` -- the shared path behind `/Artists`, `/AlbumArtists`, `/Genres`, `/MusicGenres` and `/Studios` -- disabled the total record count whenever the query carried no `Limit`: if (!filter.Limit.HasValue) { filter.EnableTotalRecordCount = false; } A request without an explicit limit therefore came back with N entries in `Items` and `TotalRecordCount = 0`. Clients that page on the reported total -- the documented contract every other list endpoint honours -- read that as an empty library. `/Items` and `/Persons` do not share this path and report the count correctly, which is what makes the inconsistency visible from the outside. Measured against master with a 62-track music library: GET /Artists?UserId=... -> TotalRecordCount=0 Items=5 GET /Artists?UserId=...&limit=100 -> TotalRecordCount=5 Items=5 Dropping the block costs nothing: `representativeIds` is materialised into a `List` a few lines below regardless, so `.Count` was already available and the count is now reported from it. Callers that genuinely want to skip the count still can -- `EnableTotalRecordCount = false` is honoured as before. The block also mutated the caller's own query object, so a query instance reused across calls silently lost its total after the first limitless one. That is covered by a test as well. --- .../BaseItemRepositoryByNameTotalCountTests.cs | 199 +++++++++++++++++++++ 1 file changed, 199 insertions(+) create mode 100644 tests/Jellyfin.Server.Implementations.Tests/Item/BaseItemRepositoryByNameTotalCountTests.cs (limited to 'tests/Jellyfin.Server.Implementations.Tests/Item/BaseItemRepositoryByNameTotalCountTests.cs') diff --git a/tests/Jellyfin.Server.Implementations.Tests/Item/BaseItemRepositoryByNameTotalCountTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Item/BaseItemRepositoryByNameTotalCountTests.cs new file mode 100644 index 0000000000..f675621e21 --- /dev/null +++ b/tests/Jellyfin.Server.Implementations.Tests/Item/BaseItemRepositoryByNameTotalCountTests.cs @@ -0,0 +1,199 @@ +using System; +using System.Linq; +using Emby.Server.Implementations.Data; +using Jellyfin.Database.Implementations; +using Jellyfin.Database.Implementations.Entities; +using Jellyfin.Database.Implementations.Locking; +using Jellyfin.Database.Providers.Sqlite; +using Jellyfin.Server.Implementations.Item; +using MediaBrowser.Controller; +using MediaBrowser.Controller.Configuration; +using MediaBrowser.Controller.Entities; +using MediaBrowser.Model.Configuration; +using Microsoft.Data.Sqlite; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; +using BaseItemKind = Jellyfin.Data.Enums.BaseItemKind; + +namespace Jellyfin.Server.Implementations.Tests.Item; + +/// +/// The by-name endpoints (artists, album artists, genres, studios) all funnel through +/// GetItemValues. A query without a Limit used to have its total record count +/// silently disabled, so callers got a populated Items array next to a zero total. +/// +public sealed class BaseItemRepositoryByNameTotalCountTests : IDisposable +{ + private readonly SqliteConnection _connection; + private readonly DbContextOptions _dbOptions; + private readonly BaseItemRepository _repository; + private readonly ItemTypeLookup _itemTypeLookup; + + public BaseItemRepositoryByNameTotalCountTests() + { + _connection = new SqliteConnection("Data Source=:memory:"); + _connection.Open(); + + _dbOptions = new DbContextOptionsBuilder() + .UseSqlite(_connection) + .Options; + + using (var ctx = CreateDbContext()) + { + ctx.Database.EnsureCreated(); + } + + var factory = new Mock>(); + factory.Setup(f => f.CreateDbContext()).Returns(CreateDbContext); + + _itemTypeLookup = new ItemTypeLookup(); + + var serverConfigurationManager = new Mock(); + serverConfigurationManager.Setup(c => c.Configuration).Returns(new ServerConfiguration()); + + _repository = new BaseItemRepository( + factory.Object, + new Mock().Object, + _itemTypeLookup, + serverConfigurationManager.Object, + NullLogger.Instance); + } + + public void Dispose() + { + _connection.Dispose(); + } + + [Fact] + public void GetArtists_WithoutLimit_ReportsTotalRecordCount() + { + SeedArtists(3); + + var result = _repository.GetArtists(CreateQuery(limit: null)); + + Assert.Equal(3, result.Items.Count); + Assert.Equal(3, result.TotalRecordCount); + } + + [Fact] + public void GetArtists_WithLimit_ReportsTotalBeyondThePage() + { + SeedArtists(3); + + var result = _repository.GetArtists(CreateQuery(limit: 2)); + + Assert.Equal(2, result.Items.Count); + Assert.Equal(3, result.TotalRecordCount); + } + + [Fact] + public void GetArtists_TotalRecordCountDisabled_StaysZero() + { + SeedArtists(3); + + var query = CreateQuery(limit: null); + query.EnableTotalRecordCount = false; + + var result = _repository.GetArtists(query); + + Assert.Equal(3, result.Items.Count); + Assert.Equal(0, result.TotalRecordCount); + } + + [Fact] + public void GetArtists_WithoutLimit_DoesNotMutateCallerQuery() + { + SeedArtists(1); + + var query = CreateQuery(limit: null); + Assert.True(query.EnableTotalRecordCount); + + _repository.GetArtists(query); + + // The repository used to flip this flag on the caller's own query object, so a + // reused query silently lost its total on every subsequent call. + Assert.True(query.EnableTotalRecordCount); + } + + private static InternalItemsQuery CreateQuery(int? limit) + { + return new InternalItemsQuery(new User("test", "auth", "reset")) + { + Limit = limit + }; + } + + /// + /// Creates artists, each credited on one song, which is what + /// makes them visible to the item-value join behind the by-name endpoints. + /// + private void SeedArtists(int count) + { + using var ctx = CreateDbContext(); + + for (var i = 0; i < count; i++) + { + var name = $"Artist {i}"; + var cleanName = name.ToLowerInvariant(); + + var artistId = Guid.Parse($"aaaaaaaa-0000-0000-0000-{i:D12}"); + var songId = Guid.Parse($"55555555-0000-0000-0000-{i:D12}"); + var valueId = Guid.Parse($"cccccccc-0000-0000-0000-{i:D12}"); + + var artist = new BaseItemEntity + { + Id = artistId, + Type = _itemTypeLookup.BaseItemKindNames[BaseItemKind.MusicArtist], + Name = name, + CleanName = cleanName, + PresentationUniqueKey = artistId.ToString("N"), + IsFolder = true, + IsVirtualItem = false + }; + + var song = new BaseItemEntity + { + Id = songId, + Type = _itemTypeLookup.BaseItemKindNames[BaseItemKind.Audio], + Name = $"Song {i}", + CleanName = $"song {i}", + PresentationUniqueKey = songId.ToString("N"), + MediaType = "Audio", + IsFolder = false, + IsVirtualItem = false + }; + + var itemValue = new ItemValue + { + ItemValueId = valueId, + Type = ItemValueType.Artist, + Value = name, + CleanValue = cleanName + }; + + ctx.BaseItems.Add(artist); + ctx.BaseItems.Add(song); + ctx.ItemValues.Add(itemValue); + ctx.ItemValuesMap.Add(new ItemValueMap + { + ItemId = songId, + ItemValueId = valueId, + Item = song, + ItemValue = itemValue + }); + } + + ctx.SaveChanges(); + } + + private JellyfinDbContext CreateDbContext() + { + return new JellyfinDbContext( + _dbOptions, + NullLogger.Instance, + new SqliteDatabaseProvider(null!, NullLogger.Instance), + new NoLockBehavior(NullLogger.Instance)); + } +} -- cgit v1.2.3