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

Release a folder's children once its subtree has been scanned

Original-merge: 2f2adae7d99537e53417f6d92aa3d40345a17b99

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

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

6 files changed, 268 insertions, 2 deletions

diff --git a/Jellyfin.Server.Implementations/Item/BaseItemMapper.cs b/Jellyfin.Server.Implementations/Item/BaseItemMapper.cs
index c2cb644c59..6405c8c45d 100644
--- a/Jellyfin.Server.Implementations/Item/BaseItemMapper.cs
+++ b/Jellyfin.Server.Implementations/Item/BaseItemMapper.cs
@@ -34,6 +34,40 @@ public static class BaseItemMapper
     /// </summary>
     private static readonly ConcurrentDictionary<string, Type?> _typeMap = new ConcurrentDictionary<string, Type?>();
 
+    private static UserData[] DetachUserData(BaseItemEntity entity)
+    {
+        if (entity.UserData is null || entity.UserData.Count == 0)
+        {
+            return [];
+        }
+
+        var detached = new UserData[entity.UserData.Count];
+        var index = 0;
+        foreach (var userData in entity.UserData)
+        {
+            detached[index++] = new UserData
+            {
+                ItemId = userData.ItemId,
+                Item = null,
+                UserId = userData.UserId,
+                User = null,
+                CustomDataKey = userData.CustomDataKey,
+                Rating = userData.Rating,
+                PlaybackPositionTicks = userData.PlaybackPositionTicks,
+                PlayCount = userData.PlayCount,
+                IsFavorite = userData.IsFavorite,
+                LastPlayedDate = userData.LastPlayedDate,
+                Played = userData.Played,
+                AudioStreamIndex = userData.AudioStreamIndex,
+                SubtitleStreamIndex = userData.SubtitleStreamIndex,
+                Likes = userData.Likes,
+                RetentionDate = userData.RetentionDate
+            };
+        }
+
+        return detached;
+    }
+
     /// <summary>
     /// Maps a Entity to the DTO.
     /// </summary>
@@ -87,7 +121,7 @@ public static class BaseItemMapper
         dto.OwnerId = entity.OwnerId ?? Guid.Empty;
         dto.Width = entity.Width.GetValueOrDefault();
         dto.Height = entity.Height.GetValueOrDefault();
-        dto.UserData = entity.UserData;
+        dto.UserData = DetachUserData(entity);
 
         if (entity.Provider is not null)
         {
diff --git a/MediaBrowser.Controller/Entities/Audio/MusicAlbum.cs b/MediaBrowser.Controller/Entities/Audio/MusicAlbum.cs
index d016d8f62b..281a98dad5 100644
--- a/MediaBrowser.Controller/Entities/Audio/MusicAlbum.cs
+++ b/MediaBrowser.Controller/Entities/Audio/MusicAlbum.cs
@@ -165,6 +165,18 @@ namespace MediaBrowser.Controller.Entities.Audio
 
         public async Task RefreshAllMetadata(MetadataRefreshOptions refreshOptions, IProgress<double> progress, CancellationToken cancellationToken)
         {
+            try
+            {
+                await RefreshAllMetadataInternal(refreshOptions, progress, cancellationToken).ConfigureAwait(false);
+            }
+            finally
+            {
+                ReleaseCachedChildren();
+            }
+        }
+
+        private async Task RefreshAllMetadataInternal(MetadataRefreshOptions refreshOptions, IProgress<double> progress, CancellationToken cancellationToken)
+        {
             var items = GetRecursiveChildren();
 
             var totalItems = items.Count;
diff --git a/MediaBrowser.Controller/Entities/Folder.cs b/MediaBrowser.Controller/Entities/Folder.cs
index b3e7369205..25e19c8351 100644
--- a/MediaBrowser.Controller/Entities/Folder.cs
+++ b/MediaBrowser.Controller/Entities/Folder.cs
@@ -286,6 +286,27 @@ namespace MediaBrowser.Controller.Entities
             return GetCachedChildren();
         }
 
+        /// <summary>
+        /// Drops the children this folder has materialised, and the ones held by every folder below
+        /// it, without loading anything that is not already in memory.
+        /// </summary>
+        public void ReleaseCachedChildren()
+        {
+            // Cleared before descending, so a folder already on the way down is not walked twice.
+            var children = _children;
+            _children = null;
+
+            if (children is null)
+            {
+                return;
+            }
+
+            foreach (var child in children)
+            {
+                (child as Folder)?.ReleaseCachedChildren();
+            }
+        }
+
         public override double? GetRefreshProgress()
         {
             return ProviderManager.GetRefreshProgress(Id);
@@ -370,6 +391,9 @@ namespace MediaBrowser.Controller.Entities
                 {
                     ProviderManager.OnRefreshComplete(this);
                 }
+
+                // The subtree is done with, so stop holding it.
+                ReleaseCachedChildren();
             }
         }
 
@@ -835,7 +859,14 @@ namespace MediaBrowser.Controller.Entities
                 if (recursive && child is Folder folder)
                 {
                     folder.Children = null; // invalidate cached children.
-                    await folder.RefreshMetadataRecursive(folder.Children.Except([this, child]).ToList(), refreshOptions, true, progress, cancellationToken).ConfigureAwait(false);
+                    try
+                    {
+                        await folder.RefreshMetadataRecursive(folder.Children.Except([this, child]).ToList(), refreshOptions, true, progress, cancellationToken).ConfigureAwait(false);
+                    }
+                    finally
+                    {
+                        folder.ReleaseCachedChildren();
+                    }
                 }
             }
         }
diff --git a/MediaBrowser.Controller/Entities/TV/Series.cs b/MediaBrowser.Controller/Entities/TV/Series.cs
index 1a1da84b7a..126f4361ba 100644
--- a/MediaBrowser.Controller/Entities/TV/Series.cs
+++ b/MediaBrowser.Controller/Entities/TV/Series.cs
@@ -333,6 +333,19 @@ namespace MediaBrowser.Controller.Entities.TV
         public async Task RefreshAllMetadata(MetadataRefreshOptions refreshOptions, IProgress<double> progress, CancellationToken cancellationToken)
         {
             Children = null; // invalidate cached children.
+
+            try
+            {
+                await RefreshAllMetadataInternal(refreshOptions, progress, cancellationToken).ConfigureAwait(false);
+            }
+            finally
+            {
+                ReleaseCachedChildren();
+            }
+        }
+
+        private async Task RefreshAllMetadataInternal(MetadataRefreshOptions refreshOptions, IProgress<double> progress, CancellationToken cancellationToken)
+        {
             // Refresh bottom up, seasons and episodes first, then the series
             var items = GetRecursiveChildren();
 
diff --git a/tests/Jellyfin.Controller.Tests/Entities/FolderChildCacheTests.cs b/tests/Jellyfin.Controller.Tests/Entities/FolderChildCacheTests.cs
new file mode 100644
index 0000000000..705238317a
--- /dev/null
+++ b/tests/Jellyfin.Controller.Tests/Entities/FolderChildCacheTests.cs
@@ -0,0 +1,97 @@
+using System.Collections.Generic;
+using MediaBrowser.Controller.Entities;
+using Xunit;
+
+namespace Jellyfin.Controller.Tests.Entities;
+
+/// <summary>
+/// Covers <see cref="Folder.ReleaseCachedChildren"/>, which a recursive scan calls as it unwinds so
+/// the folders it walked do not keep the whole item graph of the library alive behind it.
+/// </summary>
+public class FolderChildCacheTests
+{
+    [Fact]
+    public void ReleaseCachedChildren_MakesTheNextAccessReload()
+    {
+        var folder = new TrackingFolder();
+
+        Assert.Empty(folder.Children);
+        Assert.Equal(1, folder.LoadCount);
+
+        // Second access is served from the cache on the instance.
+        Assert.Empty(folder.Children);
+        Assert.Equal(1, folder.LoadCount);
+
+        folder.ReleaseCachedChildren();
+
+        Assert.Empty(folder.Children);
+        Assert.Equal(2, folder.LoadCount);
+    }
+
+    [Fact]
+    public void ReleaseCachedChildren_ReachesEveryLevelBelow()
+    {
+        var leaf = new TrackingFolder();
+        var middle = new TrackingFolder { Source = [leaf] };
+        var root = new TrackingFolder { Source = [middle] };
+
+        // Walk the whole tree, as a recursive scan does, so every level holds its children.
+        Assert.Single(root.Children);
+        Assert.Single(middle.Children);
+        Assert.Empty(leaf.Children);
+        Assert.Equal(1, root.LoadCount);
+        Assert.Equal(1, middle.LoadCount);
+        Assert.Equal(1, leaf.LoadCount);
+
+        root.ReleaseCachedChildren();
+
+        Assert.Single(root.Children);
+        Assert.Single(middle.Children);
+        Assert.Empty(leaf.Children);
+        Assert.Equal(2, root.LoadCount);
+        Assert.Equal(2, middle.LoadCount);
+        Assert.Equal(2, leaf.LoadCount);
+    }
+
+    [Fact]
+    public void ReleaseCachedChildren_LoadsNothingThatIsNotAlreadyHeld()
+    {
+        var leaf = new TrackingFolder();
+        var root = new TrackingFolder { Source = [leaf] };
+
+        root.ReleaseCachedChildren();
+
+        Assert.Equal(0, root.LoadCount);
+        Assert.Equal(0, leaf.LoadCount);
+    }
+
+    [Fact]
+    public void ReleaseCachedChildren_TerminatesOnACycle()
+    {
+        var first = new TrackingFolder();
+        var second = new TrackingFolder { Source = [first] };
+        first.Source = [second];
+
+        Assert.Single(first.Children);
+        Assert.Single(second.Children);
+
+        // Clearing before descending is what stops this from recursing forever.
+        first.ReleaseCachedChildren();
+
+        Assert.Equal(1, first.LoadCount);
+        Assert.Equal(1, second.LoadCount);
+    }
+
+    private sealed class TrackingFolder : Folder
+    {
+        public int LoadCount { get; private set; }
+
+        public IReadOnlyList<BaseItem> Source { get; set; } = [];
+
+        protected override IReadOnlyList<BaseItem> LoadChildren()
+        {
+            LoadCount++;
+            return Source;
+        }
+    }
+}
diff --git a/tests/Jellyfin.Server.Implementations.Tests/Item/BaseItemMapperUserDataTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Item/BaseItemMapperUserDataTests.cs
new file mode 100644
index 0000000000..abe1e59496
--- /dev/null
+++ b/tests/Jellyfin.Server.Implementations.Tests/Item/BaseItemMapperUserDataTests.cs
@@ -0,0 +1,79 @@
+using System;
+using System.Linq;
+using Jellyfin.Database.Implementations.Entities;
+using Jellyfin.Server.Implementations.Item;
+using MediaBrowser.Controller.Entities;
+using Xunit;
+
+namespace Jellyfin.Server.Implementations.Tests.Item;
+
+/// <summary>
+/// Covers the user data rows <see cref="BaseItemMapper"/> hands the domain item. A domain item is
+/// held for as long as its folder holds it, so a row that still points back at the entity it was
+/// read with would keep that entity - and everything loaded alongside it - alive with it.
+/// </summary>
+public class BaseItemMapperUserDataTests
+{
+    [Fact]
+    public void Map_CopiesUserDataWithoutTheEntityGraphBehindIt()
+    {
+        var itemId = Guid.NewGuid();
+        var userId = Guid.NewGuid();
+        var user = new User("someone", "Default", "Default");
+        var entity = new BaseItemEntity { Id = itemId, Type = "MediaBrowser.Controller.Entities.TV.Episode" };
+
+        var row = new UserData
+        {
+            ItemId = itemId,
+            Item = entity,
+            UserId = userId,
+            User = user,
+            CustomDataKey = "key",
+            PlayCount = 3,
+            PlaybackPositionTicks = 1234,
+            IsFavorite = true,
+            Played = true,
+            Rating = 7.5,
+            LastPlayedDate = new DateTime(2026, 9, 8, 0, 0, 0, DateTimeKind.Utc),
+            AudioStreamIndex = 1,
+            SubtitleStreamIndex = 2,
+            Likes = true
+        };
+
+        entity.UserData = [row];
+
+        var dto = BaseItemMapper.Map(entity, new Folder(), null);
+
+        var mapped = Assert.Single(dto.UserData);
+        Assert.Null(mapped.Item);
+        Assert.Null(mapped.User);
+
+        // The values callers actually read still come through.
+        Assert.Equal(itemId, mapped.ItemId);
+        Assert.Equal(userId, mapped.UserId);
+        Assert.Equal("key", mapped.CustomDataKey);
+        Assert.Equal(3, mapped.PlayCount);
+        Assert.Equal(1234, mapped.PlaybackPositionTicks);
+        Assert.True(mapped.IsFavorite);
+        Assert.True(mapped.Played);
+        Assert.Equal(7.5, mapped.Rating);
+        Assert.Equal(row.LastPlayedDate, mapped.LastPlayedDate);
+        Assert.Equal(1, mapped.AudioStreamIndex);
+        Assert.Equal(2, mapped.SubtitleStreamIndex);
+        Assert.True(mapped.Likes);
+    }
+
+    [Fact]
+    public void Map_WithoutUserData_YieldsAnEmptyCollection()
+    {
+        var entity = new BaseItemEntity
+        {
+            Id = Guid.NewGuid(),
+            Type = "MediaBrowser.Controller.Entities.Folder"
+        };
+
+        var dto = BaseItemMapper.Map(entity, new Folder(), null);
+
+        Assert.Empty(dto.UserData);
+    }
+}