Collapse version groups to the primary version in queries

2 files changed, 151 insertions, 4 deletions

diff --git a/Jellyfin.Server.Implementations/Item/BaseItemRepository.QueryBuilding.cs b/Jellyfin.Server.Implementations/Item/BaseItemRepository.QueryBuilding.cs
index d6ddf8f5c8..decd45ae2c 100644
--- a/Jellyfin.Server.Implementations/Item/BaseItemRepository.QueryBuilding.cs
+++ b/Jellyfin.Server.Implementations/Item/BaseItemRepository.QueryBuilding.cs
@@ -62,18 +62,21 @@ public sealed partial class BaseItemRepository
 
     private IQueryable<BaseItemEntity> ApplyGroupingFilter(JellyfinDbContext context, IQueryable<BaseItemEntity> dbQuery, InternalItemsQuery filter)
     {
-        // Collapse duplicates sharing a presentation key (e.g. alternate versions) by picking
-        // the min Id per group. Keep the grouped ids as an IQueryable sub-select; materializing
+        // Collapse duplicates sharing a presentation key (e.g. alternate versions), preferring the
+        // 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.
         var enableGroupByPresentationUniqueKey = EnableGroupByPresentationUniqueKey(filter);
         if (enableGroupByPresentationUniqueKey && filter.GroupBySeriesPresentationUniqueKey)
         {
-            var groupedIds = dbQuery.GroupBy(e => new { e.PresentationUniqueKey, e.SeriesPresentationUniqueKey }).Select(e => e.Min(x => x.Id));
+            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));
             dbQuery = context.BaseItems.AsNoTracking().Where(e => groupedIds.Contains(e.Id));
         }
         else if (enableGroupByPresentationUniqueKey)
         {
-            var groupedIds = dbQuery.GroupBy(e => e.PresentationUniqueKey).Select(e => e.Min(x => x.Id));
+            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));
             dbQuery = context.BaseItems.AsNoTracking().Where(e => groupedIds.Contains(e.Id));
         }
         else if (filter.GroupBySeriesPresentationUniqueKey)
diff --git a/tests/Jellyfin.Server.Implementations.Tests/Item/BaseItemRepositoryGroupingTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Item/BaseItemRepositoryGroupingTests.cs
new file mode 100644
index 0000000000..083f725db9
--- /dev/null
+++ b/tests/Jellyfin.Server.Implementations.Tests/Item/BaseItemRepositoryGroupingTests.cs
@@ -0,0 +1,144 @@
+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 MediaBrowser.Model.Entities;
+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;
+
+public sealed class BaseItemRepositoryGroupingTests : IDisposable
+{
+    private readonly SqliteConnection _connection;
+    private readonly DbContextOptions<JellyfinDbContext> _dbOptions;
+    private readonly BaseItemRepository _repository;
+    private readonly string _movieTypeName;
+
+    public BaseItemRepositoryGroupingTests()
+    {
+        _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);
+
+        var itemTypeLookup = new ItemTypeLookup();
+        _movieTypeName = itemTypeLookup.BaseItemKindNames[BaseItemKind.Movie];
+
+        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 GetItemList_VersionGroup_ReturnsPrimaryVersion()
+    {
+        // The alternate version sorts before the primary by id, so a plain Min(Id) per
+        // presentation key would wrongly pick the alternate as the group representative.
+        var primaryId = Guid.Parse("eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee");
+        var alternateId = Guid.Parse("11111111-1111-1111-1111-111111111111");
+        var presentationKey = primaryId.ToString("N");
+
+        using (var ctx = CreateDbContext())
+        {
+            ctx.BaseItems.Add(CreateMovieEntity(primaryId, "Movie", presentationKey, null));
+            ctx.BaseItems.Add(CreateMovieEntity(alternateId, "Movie - 1080p", presentationKey, primaryId));
+            ctx.SaveChanges();
+        }
+
+        var result = _repository.GetItemList(CreateQuery());
+
+        var item = Assert.Single(result);
+        Assert.Equal(primaryId, item.Id);
+    }
+
+    [Fact]
+    public void GetItemList_GroupWithoutPrimary_FallsBackToMinId()
+    {
+        var firstId = Guid.Parse("22222222-2222-2222-2222-222222222222");
+        var secondId = Guid.Parse("dddddddd-dddd-dddd-dddd-dddddddddddd");
+        var otherPrimaryId = Guid.Parse("cccccccc-cccc-cccc-cccc-cccccccccccc");
+        var presentationKey = otherPrimaryId.ToString("N");
+
+        using (var ctx = CreateDbContext())
+        {
+            ctx.BaseItems.Add(CreateMovieEntity(firstId, "Movie", presentationKey, otherPrimaryId));
+            ctx.BaseItems.Add(CreateMovieEntity(secondId, "Movie - 4K", presentationKey, otherPrimaryId));
+            ctx.SaveChanges();
+        }
+
+        var result = _repository.GetItemList(CreateQuery());
+
+        var item = Assert.Single(result);
+        Assert.Equal(firstId, item.Id);
+    }
+
+    private static InternalItemsQuery CreateQuery()
+    {
+        // IncludeOwnedItems keeps the alternate version rows in the query so the
+        // grouping collapse is what picks the group representative.
+        return new InternalItemsQuery(new Database.Implementations.Entities.User("test", "auth", "reset"))
+        {
+            IncludeItemTypes = [BaseItemKind.Movie],
+            IncludeOwnedItems = true
+        };
+    }
+
+    private BaseItemEntity CreateMovieEntity(Guid id, string name, string presentationKey, Guid? primaryVersionId)
+    {
+        return new BaseItemEntity
+        {
+            Id = id,
+            Type = _movieTypeName,
+            Name = name,
+            PresentationUniqueKey = presentationKey,
+            PrimaryVersionId = primaryVersionId,
+            MediaType = "Video",
+            IsMovie = true,
+            IsFolder = false,
+            IsVirtualItem = false
+        };
+    }
+
+    private JellyfinDbContext CreateDbContext()
+    {
+        return new JellyfinDbContext(
+            _dbOptions,
+            NullLogger<JellyfinDbContext>.Instance,
+            new SqliteDatabaseProvider(null!, NullLogger<SqliteDatabaseProvider>.Instance),
+            new NoLockBehavior(NullLogger<NoLockBehavior>.Instance));
+    }
+}