diff --git a/Jellyfin.Server.Implementations/Item/BaseItemRepository.ByName.cs b/Jellyfin.Server.Implementations/Item/BaseItemRepository.ByName.cs index cdc8744642..2a4b9c516b 100644 --- a/Jellyfin.Server.Implementations/Item/BaseItemRepository.ByName.cs +++ b/Jellyfin.Server.Implementations/Item/BaseItemRepository.ByName.cs @@ -219,9 +219,11 @@ public sealed partial class BaseItemRepository } else { + // The representative is the row no other row in its group sorts before, not MIN(Id): + // PostgreSQL has no min(uuid) aggregate, while comparing two uuids is supported everywhere. representativeIds = masterQuery - .GroupBy(e => e.PresentationUniqueKey) - .Select(g => g.Min(e => e.Id)) + .Where(e => !masterQuery.Any(o => o.PresentationUniqueKey == e.PresentationUniqueKey && o.Id.CompareTo(e.Id) < 0)) + .Select(e => e.Id) .ToList(); } diff --git a/Jellyfin.Server.Implementations/Item/BaseItemRepository.QueryBuilding.cs b/Jellyfin.Server.Implementations/Item/BaseItemRepository.QueryBuilding.cs index c0067d8392..99f3577725 100644 --- a/Jellyfin.Server.Implementations/Item/BaseItemRepository.QueryBuilding.cs +++ b/Jellyfin.Server.Implementations/Item/BaseItemRepository.QueryBuilding.cs @@ -96,22 +96,36 @@ public sealed partial class BaseItemRepository // primary version (PrimaryVersionId is null) so detail pages and actions target it instead // of an arbitrary alternate. Keep the grouped ids as an IQueryable sub-select; materializing // to a List would inline one bound parameter per id and hit SQLite's variable cap. + // The representative is the row no other row in its group sorts before, not MIN(Id): + // PostgreSQL has no min(uuid) aggregate, while comparing two uuids is supported everywhere. + // The anti-join reads the filtered set twice, so it has to close over a local that the + // reassignment below cannot reach - capturing dbQuery itself makes the tree self-referential. + var candidates = dbQuery; var enableGroupByPresentationUniqueKey = EnableGroupByPresentationUniqueKey(filter); if (enableGroupByPresentationUniqueKey && filter.GroupBySeriesPresentationUniqueKey) { - var groupedIds = dbQuery.GroupBy(e => new { e.PresentationUniqueKey, e.SeriesPresentationUniqueKey }) - .Select(g => g.Where(e => e.PrimaryVersionId == null).Min(e => (Guid?)e.Id) ?? g.Min(e => (Guid?)e.Id)); + var groupedIds = candidates + .Where(e => !candidates.Any(o => o.PresentationUniqueKey == e.PresentationUniqueKey + && o.SeriesPresentationUniqueKey == e.SeriesPresentationUniqueKey + && ((o.PrimaryVersionId == null && e.PrimaryVersionId != null) + || ((o.PrimaryVersionId == null) == (e.PrimaryVersionId == null) && o.Id.CompareTo(e.Id) < 0)))) + .Select(e => e.Id); dbQuery = context.BaseItems.AsNoTracking().Where(e => groupedIds.Contains(e.Id)); } else if (enableGroupByPresentationUniqueKey) { - var groupedIds = dbQuery.GroupBy(e => e.PresentationUniqueKey) - .Select(g => g.Where(e => e.PrimaryVersionId == null).Min(e => (Guid?)e.Id) ?? g.Min(e => (Guid?)e.Id)); + var groupedIds = candidates + .Where(e => !candidates.Any(o => o.PresentationUniqueKey == e.PresentationUniqueKey + && ((o.PrimaryVersionId == null && e.PrimaryVersionId != null) + || ((o.PrimaryVersionId == null) == (e.PrimaryVersionId == null) && o.Id.CompareTo(e.Id) < 0)))) + .Select(e => e.Id); dbQuery = context.BaseItems.AsNoTracking().Where(e => groupedIds.Contains(e.Id)); } else if (filter.GroupBySeriesPresentationUniqueKey) { - var groupedIds = dbQuery.GroupBy(e => e.SeriesPresentationUniqueKey).Select(e => e.Min(x => x.Id)); + var groupedIds = candidates + .Where(e => !candidates.Any(o => o.SeriesPresentationUniqueKey == e.SeriesPresentationUniqueKey && o.Id.CompareTo(e.Id) < 0)) + .Select(e => e.Id); dbQuery = context.BaseItems.AsNoTracking().Where(e => groupedIds.Contains(e.Id)); } else diff --git a/tests/Jellyfin.Server.Tests/Item/PostgreSqlPresentationKeyGroupingTests.cs b/tests/Jellyfin.Server.Tests/Item/PostgreSqlPresentationKeyGroupingTests.cs new file mode 100644 index 0000000000..343542266c --- /dev/null +++ b/tests/Jellyfin.Server.Tests/Item/PostgreSqlPresentationKeyGroupingTests.cs @@ -0,0 +1,213 @@ +using System; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Emby.Server.Implementations.Data; +using Jellyfin.Database.Implementations; +using Jellyfin.Database.Implementations.DbConfiguration; +using Jellyfin.Database.Implementations.Entities; +using Jellyfin.Database.Implementations.Locking; +using Jellyfin.Database.Providers.PostgreSQL; +using Jellyfin.Server.Implementations.Item; +using Jellyfin.Server.Tests.Migrations; +using MediaBrowser.Controller; +using MediaBrowser.Controller.Configuration; +using MediaBrowser.Controller.Entities; +using MediaBrowser.Model.Configuration; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Npgsql; +using Xunit; +using BaseItemKind = Jellyfin.Data.Enums.BaseItemKind; +using User = Jellyfin.Database.Implementations.Entities.User; + +namespace Jellyfin.Server.Tests.Item; + +/// +/// Runs the presentation-key collapse that library browse, search and the by-name endpoints all go +/// through against a real PostgreSQL. SQLite accepts MIN over any column type, PostgreSQL has no +/// min(uuid) aggregate, so a representative picked with an aggregate over the id only ever fails +/// here - with 42883 function min(uuid) does not exist. +/// +[Trait("Category", "RequiresDocker")] +public sealed class PostgreSqlPresentationKeyGroupingTests : IAsyncLifetime +{ + // The alternate sorts before the primary, and the second duplicate genre before the first, so a + // representative that ignores the primary-version preference or the id order picks the wrong row. + private static readonly Guid _primaryMovieId = Guid.Parse("eeeeeeee-0000-0000-0000-000000000001"); + private static readonly Guid _alternateMovieId = Guid.Parse("11111111-0000-0000-0000-000000000002"); + private static readonly Guid _secondAlternateMovieId = Guid.Parse("22222222-0000-0000-0000-000000000003"); + private static readonly Guid _firstOrphanId = Guid.Parse("33333333-0000-0000-0000-000000000004"); + private static readonly Guid _secondOrphanId = Guid.Parse("dddddddd-0000-0000-0000-000000000005"); + private static readonly Guid _orphanPrimaryId = Guid.Parse("cccccccc-0000-0000-0000-000000000006"); + private static readonly Guid _standaloneMovieId = Guid.Parse("44444444-0000-0000-0000-000000000007"); + + private static readonly Guid _firstGenreId = Guid.Parse("bbbbbbbb-0000-0000-0000-000000000001"); + private static readonly Guid _secondGenreId = Guid.Parse("aaaaaaaa-0000-0000-0000-000000000002"); + private static readonly Guid _genreMovieId = Guid.Parse("66666666-0000-0000-0000-000000000003"); + private static readonly Guid _genreValueId = Guid.Parse("77777777-0000-0000-0000-000000000004"); + + private readonly ItemTypeLookup _itemTypeLookup = new(); + + private PostgreSqlTestServer _server = null!; + private NpgsqlDataSource _dataSource = null!; + private BaseItemRepository _repository = null!; + + public async ValueTask InitializeAsync() + { + _server = await PostgreSqlTestServer.StartAsync().ConfigureAwait(false); + var connectionString = await _server.CreateDatabaseAsync("presentation_key_grouping", TestContext.Current.CancellationToken).ConfigureAwait(false); + _dataSource = new NpgsqlDataSourceBuilder(connectionString).Build(); + + var context = CreateDbContext(); + await using (context.ConfigureAwait(false)) + { + await context.Database.EnsureCreatedAsync(TestContext.Current.CancellationToken).ConfigureAwait(false); + } + + var serverConfigurationManager = new Mock(); + serverConfigurationManager.Setup(c => c.Configuration).Returns(new ServerConfiguration()); + + var factory = new Mock>(); + factory.Setup(f => f.CreateDbContext()).Returns(CreateDbContext); + factory.Setup(f => f.CreateDbContextAsync(It.IsAny())).ReturnsAsync(CreateDbContext); + + _repository = new BaseItemRepository( + factory.Object, + new Mock().Object, + _itemTypeLookup, + serverConfigurationManager.Object, + NullLogger.Instance); + + await SeedAsync().ConfigureAwait(false); + } + + public async ValueTask DisposeAsync() + { + await _dataSource.DisposeAsync().ConfigureAwait(false); + await _server.DisposeAsync().ConfigureAwait(false); + } + + /// + /// The collapse behind library browse and search: one row per presentation key, the primary version + /// when the group has one, the lowest id otherwise. + /// + [Fact] + public void GetItemList_CollapsesPresentationKeyGroups() + { + var items = _repository.GetItemList(new InternalItemsQuery(new User("grouping", "auth", "reset")) + { + IncludeItemTypes = [BaseItemKind.Movie], + IncludeOwnedItems = true + }); + + Assert.Equal( + new[] { _primaryMovieId, _firstOrphanId, _standaloneMovieId, _genreMovieId }.OrderBy(id => id).ToArray(), + items.Select(i => i.Id).OrderBy(id => id).ToArray()); + } + + /// + /// The same collapse on the by-name path, which picks the lowest id per group without a + /// primary-version preference. + /// + [Fact] + public void GetGenres_CollapsesPresentationKeyGroups() + { + var result = _repository.GetGenres(new InternalItemsQuery(new User("genres", "auth", "reset"))); + + var item = Assert.Single(result.Items); + Assert.Equal(_secondGenreId, item.Item.Id); + Assert.Equal(1, result.TotalRecordCount); + } + + private JellyfinDbContext CreateDbContext() + { + var optionsBuilder = new DbContextOptionsBuilder(); + var provider = new PostgreSqlDatabaseProvider(_dataSource); + provider.Initialise(optionsBuilder, new DatabaseConfigurationOptions { DatabaseType = "PostgreSQL" }); + return new JellyfinDbContext( + optionsBuilder.Options, + NullLogger.Instance, + provider, + new NoLockBehavior(NullLogger.Instance)); + } + + private async Task SeedAsync() + { + var context = CreateDbContext(); + await using (context.ConfigureAwait(false)) + { + // One version group: a primary plus two alternates, both sorting before it by id. + var versionKey = _primaryMovieId.ToString("N"); + context.BaseItems.Add(CreateMovie(_primaryMovieId, "Movie", versionKey, null)); + context.BaseItems.Add(CreateMovie(_alternateMovieId, "Movie - 1080p", versionKey, _primaryMovieId)); + context.BaseItems.Add(CreateMovie(_secondAlternateMovieId, "Movie - 4K", versionKey, _primaryMovieId)); + + // One group whose primary is not in the result set, so the lowest id represents it. + var orphanKey = _orphanPrimaryId.ToString("N"); + context.BaseItems.Add(CreateMovie(_firstOrphanId, "Orphan - 1080p", orphanKey, _orphanPrimaryId)); + context.BaseItems.Add(CreateMovie(_secondOrphanId, "Orphan - 4K", orphanKey, _orphanPrimaryId)); + + context.BaseItems.Add(CreateMovie(_standaloneMovieId, "Standalone", _standaloneMovieId.ToString("N"), null)); + + // Two genre entities sharing a presentation key, credited on one movie so the by-name + // item-value join sees them. + var genreKey = _firstGenreId.ToString("N"); + context.BaseItems.Add(CreateGenre(_firstGenreId, "Action", genreKey)); + context.BaseItems.Add(CreateGenre(_secondGenreId, "Action", genreKey)); + + var genreMovie = CreateMovie(_genreMovieId, "Genre Movie", _genreMovieId.ToString("N"), null); + genreMovie.CleanName = "genre movie"; + context.BaseItems.Add(genreMovie); + + var genreValue = new ItemValue + { + ItemValueId = _genreValueId, + Type = ItemValueType.Genre, + Value = "Action", + CleanValue = "action" + }; + context.ItemValues.Add(genreValue); + context.ItemValuesMap.Add(new ItemValueMap + { + ItemId = _genreMovieId, + ItemValueId = _genreValueId, + Item = genreMovie, + ItemValue = genreValue + }); + + await context.SaveChangesAsync(TestContext.Current.CancellationToken).ConfigureAwait(false); + } + } + + private BaseItemEntity CreateMovie(Guid id, string name, string presentationKey, Guid? primaryVersionId) + { + return new BaseItemEntity + { + Id = id, + Type = _itemTypeLookup.BaseItemKindNames[BaseItemKind.Movie], + Name = name, + PresentationUniqueKey = presentationKey, + PrimaryVersionId = primaryVersionId, + MediaType = "Video", + IsMovie = true, + IsFolder = false, + IsVirtualItem = false + }; + } + + private BaseItemEntity CreateGenre(Guid id, string name, string presentationKey) + { + return new BaseItemEntity + { + Id = id, + Type = _itemTypeLookup.BaseItemKindNames[BaseItemKind.Genre], + Name = name, + CleanName = "action", + PresentationUniqueKey = presentationKey, + IsFolder = false, + IsVirtualItem = false + }; + } +}