diff options
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); + } +} |