Backport pull request #17881 from jellyfin/release-12.z

Fix /UserViews exhausting memory and reporting random child counts

Original-merge: a838a06aa51eac388a76e9e6421f1a80873c417b

Merged-by: crobibero <cody@robibe.ro>

Backported-by: Cody Robibero <cody@robibe.ro>

4 files changed, 431 insertions, 0 deletions

diff --git a/tests/Jellyfin.Controller.Tests/Entities/PlaylistTests.cs b/tests/Jellyfin.Controller.Tests/Entities/PlaylistTests.cs
new file mode 100644
index 0000000000..70da5eafe5
--- /dev/null
+++ b/tests/Jellyfin.Controller.Tests/Entities/PlaylistTests.cs
@@ -0,0 +1,84 @@
+using System;
+using System.Linq;
+using Jellyfin.Data;
+using Jellyfin.Data.Enums;
+using Jellyfin.Database.Implementations.Entities;
+using Jellyfin.Database.Implementations.Enums;
+using MediaBrowser.Controller.Entities;
+using MediaBrowser.Controller.Entities.Audio;
+using MediaBrowser.Controller.Entities.Movies;
+using MediaBrowser.Controller.Library;
+using MediaBrowser.Controller.Playlists;
+using MediaBrowser.Model.Querying;
+using Moq;
+using Xunit;
+
+namespace Jellyfin.Controller.Tests.Entities;
+
+public class PlaylistTests
+{
+    [Fact]
+    public void IsVisible_PlaylistWithNothingLeftInIt_IsHidden()
+    {
+        // The SQL parental filter hides a container whose every member is blocked, so a listing
+        // built in memory has to reach the same answer.
+        var blocked = new Movie { Id = Guid.NewGuid(), Name = "Movie" };
+        SetupLibrary(blocked);
+
+        Assert.False(BuildPlaylist(blocked).IsVisible(BuildRestrictedUser()));
+    }
+
+    [Fact]
+    public void IsVisible_PlaylistWithOneAllowedItem_StaysVisible()
+    {
+        var blocked = new Movie { Id = Guid.NewGuid(), Name = "Movie" };
+        var allowed = new Audio { Id = Guid.NewGuid(), Name = "Song" };
+        SetupLibrary(blocked, allowed);
+
+        Assert.True(BuildPlaylist(blocked, allowed).IsVisible(BuildRestrictedUser()));
+    }
+
+    [Fact]
+    public void IsVisible_UnrestrictedUser_LeavesTheItemsUnresolved()
+    {
+        var blocked = new Movie { Id = Guid.NewGuid(), Name = "Movie" };
+        var libraryManager = SetupLibrary(blocked);
+        var user = new User("user", "auth-provider", "reset-provider");
+
+        Assert.True(BuildPlaylist(blocked).IsVisible(user));
+
+        // Resolving a playlist's items is a query per playlist; nothing may run it for a user no
+        // rating keeps anything from.
+        libraryManager.Verify(x => x.GetItemList(It.IsAny<InternalItemsQuery>()), Times.Never);
+    }
+
+    private static Mock<ILibraryManager> SetupLibrary(params BaseItem[] items)
+    {
+        var libraryManager = new Mock<ILibraryManager>();
+        libraryManager
+            .Setup(x => x.GetItemList(It.IsAny<InternalItemsQuery>()))
+            .Returns(items);
+        BaseItem.LibraryManager = libraryManager.Object;
+
+        return libraryManager;
+    }
+
+    private static Playlist BuildPlaylist(params BaseItem[] items)
+    {
+        // An empty path keeps the playlist out of the shared-playlist branch.
+        return new Playlist
+        {
+            Id = Guid.NewGuid(),
+            Name = "Playlist",
+            LinkedChildren = items.Select(LinkedChild.Create).ToArray()
+        };
+    }
+
+    private static User BuildRestrictedUser()
+    {
+        var user = new User("user", "auth-provider", "reset-provider") { MaxParentalRatingScore = 5 };
+        user.SetPreference(PreferenceKind.BlockUnratedItems, new[] { UnratedItem.Movie });
+
+        return user;
+    }
+}
diff --git a/tests/Jellyfin.Server.Implementations.Tests/Dto/DtoServiceTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Dto/DtoServiceTests.cs
index 679e6d17e3..fd84cfb497 100644
--- a/tests/Jellyfin.Server.Implementations.Tests/Dto/DtoServiceTests.cs
+++ b/tests/Jellyfin.Server.Implementations.Tests/Dto/DtoServiceTests.cs
@@ -1,7 +1,11 @@
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using Emby.Server.Implementations.Dto;
+using Jellyfin.Data;
+using Jellyfin.Data.Enums;
 using Jellyfin.Database.Implementations.Entities;
+using Jellyfin.Database.Implementations.Enums;
 using MediaBrowser.Common;
 using MediaBrowser.Controller.Chapters;
 using MediaBrowser.Controller.Drawing;
@@ -138,6 +142,96 @@ public class DtoServiceTests
         Assert.Equal(9, dto.ChildCount);
     }
 
+    [Fact]
+    public void GetBaseItemDtos_NoUser_SkipsTheChildCountBatch()
+    {
+        // A child count is attached only to a user's dto, so with no user the batch is work whose
+        // result nothing reads - and it is a grouped count over every item, not a cheap one.
+        var (season, _) = BuildSeason(playedCount: 0, totalCount: 0, childCount: 10);
+        var options = new DtoOptions(false) { EnableImages = false, Fields = [ItemFields.ChildCount] };
+
+        var dto = _dtoService.GetBaseItemDtos([season], options, user: null, skipVisibilityCheck: true)[0];
+
+        Assert.Null(dto.ChildCount);
+        _libraryManagerMock.Verify(
+            x => x.GetChildCountBatch(It.IsAny<IReadOnlyList<Guid>>(), It.IsAny<User?>()),
+            Times.Never);
+    }
+
+    [Fact]
+    public void GetBaseItemDtos_GroupedMoviesView_CountsEveryLibraryGroupedIntoIt()
+    {
+        // The view has no library of its own, so its count is the sum over the libraries the user
+        // grouped into it - including an untyped one, which the view also shows.
+        var user = new User("user", "auth-provider", "reset-provider");
+        var grouped = BuildLibrary(CollectionType.movies);
+        var untyped = BuildLibrary(null);
+        var shows = BuildLibrary(CollectionType.tvshows);
+        var ungrouped = BuildLibrary(CollectionType.movies);
+        user.SetPreference(PreferenceKind.GroupedFolders, [grouped.Id, untyped.Id, shows.Id]);
+
+        // A real root folder would resolve its children through the library it does not have here.
+        var rootFolder = new Mock<Folder>();
+        rootFolder
+            .Setup(x => x.GetChildren(user, true, It.IsAny<InternalItemsQuery>()))
+            .Returns<User, bool, InternalItemsQuery>((_, _, _) => [grouped, untyped, shows, ungrouped]);
+        _libraryManagerMock.Setup(x => x.GetUserRootFolder()).Returns(rootFolder.Object);
+
+        IReadOnlyList<Guid>? counted = null;
+        _libraryManagerMock
+            .Setup(x => x.GetChildCountBatch(It.IsAny<IReadOnlyList<Guid>>(), It.IsAny<User?>()))
+            .Callback<IReadOnlyList<Guid>, User?>((ids, _) => counted = ids)
+            .Returns<IReadOnlyList<Guid>, User?>((ids, _) => ids.ToDictionary(id => id, _ => 4));
+
+        var view = new UserView { Id = Guid.NewGuid(), Name = "Movies", ViewType = CollectionType.movies };
+        var options = new DtoOptions(false) { EnableImages = false, Fields = [ItemFields.ChildCount] };
+
+        var dto = _dtoService.GetBaseItemDtos([view], options, user, skipVisibilityCheck: true)[0];
+
+        Assert.Equal(grouped.PhysicalFolderIds.Concat(untyped.PhysicalFolderIds), counted);
+        Assert.Equal(16, dto.ChildCount);
+    }
+
+    [Fact]
+    public void GetBaseItemDtos_SubViewOfALibrary_DoesNotCountTheLibrary()
+    {
+        // A sub-view hangs off the library the view was built over, but it holds a query over it,
+        // not its children: counting the library would report every movie as "Continue Watching".
+        var user = new User("user", "auth-provider", "reset-provider");
+        var library = BuildLibrary(CollectionType.movies);
+        _libraryManagerMock.Setup(x => x.GetItemById(library.Id)).Returns(library);
+
+        // The fallback count a sub-view falls through to runs a query of its own.
+        _libraryManagerMock
+            .Setup(x => x.GetItemList(It.IsAny<InternalItemsQuery>()))
+            .Returns([]);
+
+        var subView = new UserView
+        {
+            Id = Guid.NewGuid(),
+            Name = "Continue Watching",
+            ViewType = CollectionType.movieresume,
+            DisplayParentId = library.Id
+        };
+        var options = new DtoOptions(false) { EnableImages = false, Fields = [ItemFields.ChildCount] };
+
+        _dtoService.GetBaseItemDtos([subView], options, user, skipVisibilityCheck: true);
+
+        _libraryManagerMock.Verify(
+            x => x.GetChildCountBatch(It.IsAny<IReadOnlyList<Guid>>(), It.IsAny<User?>()),
+            Times.Never);
+    }
+
+    private static CollectionFolder BuildLibrary(CollectionType? collectionType)
+    {
+        return new CollectionFolder
+        {
+            Id = Guid.NewGuid(),
+            CollectionType = collectionType,
+            PhysicalFolderIds = [Guid.NewGuid(), Guid.NewGuid()]
+        };
+    }
+
     private (Season Season, User User) BuildSeason(int playedCount, int totalCount, int childCount)
     {
         var user = new User("user", "auth-provider", "reset-provider");
diff --git a/tests/Jellyfin.Server.Implementations.Tests/Entities/UserViewBuilderTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Entities/UserViewBuilderTests.cs
new file mode 100644
index 0000000000..54fec0a0d3
--- /dev/null
+++ b/tests/Jellyfin.Server.Implementations.Tests/Entities/UserViewBuilderTests.cs
@@ -0,0 +1,58 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using Jellyfin.Database.Implementations.Entities;
+using MediaBrowser.Controller.Entities;
+using MediaBrowser.Controller.Entities.Movies;
+using MediaBrowser.Controller.Library;
+using Moq;
+using Xunit;
+
+namespace Jellyfin.Server.Implementations.Tests.Entities;
+
+public sealed class UserViewBuilderTests
+{
+    private static readonly User _user = new("view-filter-test", "provider", "reset");
+
+    [Fact]
+    public void Filter_IsPlayed_CountsAMovieWatchedOnAnAlternateVersionAsPlayed()
+    {
+        // The primary carries no played row of its own; the version that was watched is another file.
+        var onlyWatchedOnAlternate = new Movie { Id = Guid.NewGuid(), Name = "Watched as a second cut" };
+        var watched = new Movie { Id = Guid.NewGuid(), Name = "Watched outright" };
+        var unwatched = new Movie { Id = Guid.NewGuid(), Name = "Not watched" };
+
+        var items = new BaseItem[] { onlyWatchedOnAlternate, watched, unwatched };
+
+        var userDataManager = new Mock<IUserDataManager>();
+        userDataManager
+            .Setup(m => m.GetUserData(_user, It.IsAny<BaseItem>()))
+            .Returns((User _, BaseItem item) => new UserItemData { Key = item.Id.ToString("N"), Played = item.Id.Equals(watched.Id) });
+        userDataManager
+            .Setup(m => m.GetResumeUserDataBatch(It.IsAny<IReadOnlyList<BaseItem>>(), _user))
+            .Returns(new Dictionary<Guid, VersionResumeData>
+            {
+                [onlyWatchedOnAlternate.Id] = new(Guid.NewGuid(), new UserItemData { Key = "alternate", Played = true })
+            });
+
+        var libraryManager = new Mock<ILibraryManager>();
+
+        var played = UserViewBuilder.Filter(
+            items,
+            _user,
+            new InternalItemsQuery(_user) { IsPlayed = true },
+            userDataManager.Object,
+            libraryManager.Object).ToList();
+
+        var unplayed = UserViewBuilder.Filter(
+            items,
+            _user,
+            new InternalItemsQuery(_user) { IsPlayed = false },
+            userDataManager.Object,
+            libraryManager.Object).ToList();
+
+        // The alternate's playback settles the movie, exactly as the item's own dto reports it.
+        Assert.Equal([onlyWatchedOnAlternate.Id, watched.Id], played.Select(i => i.Id));
+        Assert.Equal([unwatched.Id], unplayed.Select(i => i.Id));
+    }
+}
diff --git a/tests/Jellyfin.Server.Implementations.Tests/Item/ItemCountServiceTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Item/ItemCountServiceTests.cs
index ff683dc57a..787bb24150 100644
--- a/tests/Jellyfin.Server.Implementations.Tests/Item/ItemCountServiceTests.cs
+++ b/tests/Jellyfin.Server.Implementations.Tests/Item/ItemCountServiceTests.cs
@@ -201,6 +201,201 @@ public sealed class ItemCountServiceTests : IDisposable
     }
 
     [Fact]
+    public void GetCounts_PlayedAlternateVersion_CountThePrimaryAsPlayed()
+    {
+        var user = new User("alt-version-test", "provider", "reset");
+        var seriesId = Guid.NewGuid();
+        var primaryId = Guid.NewGuid();
+        var alternateId = Guid.NewGuid();
+
+        using (var context = CreateDbContext())
+        {
+            context.Users.Add(user);
+
+            var series = CreateItem(seriesId);
+            series.PresentationUniqueKey = "alt-version-series";
+            context.BaseItems.Add(series);
+
+            context.BaseItems.Add(CreateLeaf(primaryId));
+            var alternate = CreateLeaf(alternateId);
+            alternate.PrimaryVersionId = primaryId;
+            context.BaseItems.Add(alternate);
+            context.SaveChanges();
+
+            // Only the primary is counted as a leaf, as ApplyAccessFiltering leaves it in production.
+            AddAncestor(context, primaryId, seriesId);
+
+            context.LinkedChildren.Add(new LinkedChildEntity
+            {
+                ParentId = primaryId,
+                ChildId = alternateId,
+                ChildType = LinkedChildType.LocalAlternateVersion,
+                SortOrder = 0
+            });
+
+            // The file that was watched is the alternate, so the primary carries no played row.
+            context.UserData.Add(new UserData
+            {
+                ItemId = alternateId,
+                UserId = user.Id,
+                CustomDataKey = string.Empty,
+                Played = true,
+                Item = null,
+                User = null
+            });
+
+            context.SaveChanges();
+        }
+
+        var filter = new InternalItemsQuery(user);
+
+        // The per-item paths have to agree with the batch one, which the DTO uses interchangeably.
+        Assert.Equal(1, _service.GetPlayedCount(filter, seriesId));
+        Assert.Equal((1, 1), _service.GetPlayedAndTotalCount(filter, seriesId));
+        Assert.Equal((1, 1), _service.GetPlayedAndTotalCountBatch([seriesId], user)[seriesId]);
+    }
+
+    [Fact]
+    public void GetCounts_MultiVersionMovie_CountPlaybackOfAnyVersion()
+    {
+        // Two movies held as two files each: the primary the collection links, and an alternate version
+        // linked to it. One movie was watched on its alternate, which is where playback of a second cut
+        // lands; the other was not watched at all.
+        var user = new User("alt-version-test", "provider", "reset");
+        var boxSetId = Guid.NewGuid();
+        var libraryId = Guid.NewGuid();
+        var watchedPrimaryId = Guid.NewGuid();
+        var watchedAlternateId = Guid.NewGuid();
+        var unwatchedPrimaryId = Guid.NewGuid();
+        var unwatchedAlternateId = Guid.NewGuid();
+
+        using (var context = CreateDbContext())
+        {
+            context.Users.Add(user);
+
+            var boxSet = CreateItem(boxSetId);
+            boxSet.PresentationUniqueKey = "alt-version-box-set";
+            context.BaseItems.Add(boxSet);
+
+            var library = CreateItem(libraryId);
+            library.PresentationUniqueKey = "alt-version-library";
+            context.BaseItems.Add(library);
+
+            foreach (var (primaryId, alternateId) in
+                new[] { (watchedPrimaryId, watchedAlternateId), (unwatchedPrimaryId, unwatchedAlternateId) })
+            {
+                context.BaseItems.Add(CreateLeaf(primaryId));
+
+                var alternate = CreateLeaf(alternateId);
+                alternate.PrimaryVersionId = primaryId;
+                context.BaseItems.Add(alternate);
+            }
+
+            context.SaveChanges();
+
+            context.LinkedChildren.AddRange(
+                new LinkedChildEntity
+                {
+                    ParentId = boxSetId,
+                    ChildId = watchedPrimaryId,
+                    ChildType = LinkedChildType.Manual,
+                    SortOrder = 0
+                },
+                new LinkedChildEntity
+                {
+                    ParentId = boxSetId,
+                    ChildId = unwatchedPrimaryId,
+                    ChildType = LinkedChildType.Manual,
+                    SortOrder = 1
+                },
+                new LinkedChildEntity
+                {
+                    ParentId = watchedPrimaryId,
+                    ChildId = watchedAlternateId,
+                    ChildType = LinkedChildType.LocalAlternateVersion,
+                    SortOrder = 0
+                },
+                new LinkedChildEntity
+                {
+                    ParentId = unwatchedPrimaryId,
+                    ChildId = unwatchedAlternateId,
+                    ChildType = LinkedChildType.LocalAlternateVersion,
+                    SortOrder = 0
+                });
+
+            AddAncestor(context, watchedPrimaryId, libraryId);
+            AddAncestor(context, unwatchedPrimaryId, libraryId);
+
+            context.UserData.Add(new UserData
+            {
+                ItemId = watchedAlternateId,
+                UserId = user.Id,
+                CustomDataKey = string.Empty,
+                Played = true,
+                Item = null,
+                User = null
+            });
+
+            context.SaveChanges();
+        }
+
+        var filter = new InternalItemsQuery(user);
+
+        // A version group is one item to count, and the alternate's playback makes that item played -
+        // as it already does for the played flag the primary itself reports.
+        Assert.Equal((1, 2), _service.GetPlayedAndTotalCountFromLinkedChildren(filter, boxSetId));
+        Assert.Equal((1, 2), _service.GetPlayedAndTotalCountBatch([boxSetId], user)[boxSetId]);
+
+        // The ancestor-based paths answer the same for the library the primaries sit in.
+        Assert.Equal(1, _service.GetPlayedCount(filter, libraryId));
+        Assert.Equal((1, 2), _service.GetPlayedAndTotalCount(filter, libraryId));
+    }
+
+    [Fact]
+    public void GetChildCountBatch_NoUser_StillCollapsesAlternateVersions()
+    {
+        // Both files of a merged movie sit in the folder. With a user it is access filtering that
+        // drops the alternate; with no user nothing else would, and the folder would report two
+        // children for the one title a viewer sees.
+        var folderId = Guid.NewGuid();
+        var primaryId = Guid.NewGuid();
+        var alternateId = Guid.NewGuid();
+        var extraId = Guid.NewGuid();
+        var ownedId = Guid.NewGuid();
+
+        using (var context = CreateDbContext())
+        {
+            context.BaseItems.Add(CreateItem(folderId));
+
+            var primary = CreateLeaf(primaryId);
+            primary.ParentId = folderId;
+            context.BaseItems.Add(primary);
+
+            var alternate = CreateLeaf(alternateId);
+            alternate.ParentId = folderId;
+            alternate.PrimaryVersionId = primaryId;
+            context.BaseItems.Add(alternate);
+
+            // An extra carries an owner and an extra type, and stays a child of its own.
+            var extra = CreateLeaf(extraId);
+            extra.ParentId = folderId;
+            extra.OwnerId = primaryId;
+            extra.ExtraType = BaseItemExtraType.Trailer;
+            context.BaseItems.Add(extra);
+
+            // An owned item that is not an extra belongs to its owner, not to the folder.
+            var owned = CreateLeaf(ownedId);
+            owned.ParentId = folderId;
+            owned.OwnerId = primaryId;
+            context.BaseItems.Add(owned);
+
+            context.SaveChanges();
+        }
+
+        Assert.Equal(2, _service.GetChildCountBatch([folderId], null)[folderId]);
+    }
+
+    [Fact]
     public void GetChildCountBatch_MergedFolders_CountsDistinctChildKeys()
     {
         var seriesA = Guid.NewGuid();