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<Guid>` 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.
This commit is contained in:
@@ -144,11 +144,6 @@ public sealed partial class BaseItemRepository
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(filter);
|
||||
|
||||
if (!filter.Limit.HasValue)
|
||||
{
|
||||
filter.EnableTotalRecordCount = false;
|
||||
}
|
||||
|
||||
using var context = _dbProvider.CreateDbContext();
|
||||
|
||||
var innerQueryFilter = TranslateQuery(context.BaseItems.Where(e => e.Id != EF.Constant(PlaceholderId)), context, new InternalItemsQuery(filter.User)
|
||||
|
||||
+199
@@ -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;
|
||||
|
||||
/// <summary>
|
||||
/// The by-name endpoints (artists, album artists, genres, studios) all funnel through
|
||||
/// <c>GetItemValues</c>. A query without a <c>Limit</c> used to have its total record count
|
||||
/// silently disabled, so callers got a populated <c>Items</c> array next to a zero total.
|
||||
/// </summary>
|
||||
public sealed class BaseItemRepositoryByNameTotalCountTests : IDisposable
|
||||
{
|
||||
private readonly SqliteConnection _connection;
|
||||
private readonly DbContextOptions<JellyfinDbContext> _dbOptions;
|
||||
private readonly BaseItemRepository _repository;
|
||||
private readonly ItemTypeLookup _itemTypeLookup;
|
||||
|
||||
public BaseItemRepositoryByNameTotalCountTests()
|
||||
{
|
||||
_connection = new SqliteConnection("Data Source=:memory:");
|
||||
_connection.Open();
|
||||
|
||||
_dbOptions = new DbContextOptionsBuilder<JellyfinDbContext>()
|
||||
.UseSqlite(_connection)
|
||||
.Options;
|
||||
|
||||
using (var ctx = CreateDbContext())
|
||||
{
|
||||
ctx.Database.EnsureCreated();
|
||||
}
|
||||
|
||||
var factory = new Mock<IDbContextFactory<JellyfinDbContext>>();
|
||||
factory.Setup(f => f.CreateDbContext()).Returns(CreateDbContext);
|
||||
|
||||
_itemTypeLookup = new ItemTypeLookup();
|
||||
|
||||
var serverConfigurationManager = new Mock<IServerConfigurationManager>();
|
||||
serverConfigurationManager.Setup(c => c.Configuration).Returns(new ServerConfiguration());
|
||||
|
||||
_repository = new BaseItemRepository(
|
||||
factory.Object,
|
||||
new Mock<IServerApplicationHost>().Object,
|
||||
_itemTypeLookup,
|
||||
serverConfigurationManager.Object,
|
||||
NullLogger<BaseItemRepository>.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
|
||||
};
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Creates <paramref name="count"/> artists, each credited on one song, which is what
|
||||
/// makes them visible to the item-value join behind the by-name endpoints.
|
||||
/// </summary>
|
||||
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<JellyfinDbContext>.Instance,
|
||||
new SqliteDatabaseProvider(null!, NullLogger<SqliteDatabaseProvider>.Instance),
|
||||
new NoLockBehavior(NullLogger<NoLockBehavior>.Instance));
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user