From 5e621d0e3f2102177210f162e72a5d73378063fb Mon Sep 17 00:00:00 2001 From: Piotr Niełacny Date: Tue, 25 Aug 2026 15:19:38 +0200 Subject: Order IsPlayed and IsUnplayed by the played state the filter reports Ordering mapped both keys to the item's own stored UserData row. Folders do not have one: a series, season or box set counts as played when no descendant is left unplayed, which is what the isPlayed filter and the DTO both report. A mixed library therefore sorted every series and box set into the unplayed group, and a query could filter and sort by two different notions of "played". Extract the filter's predicate into BuildIsPlayedFilter and route both sort keys through it so the two cannot drift apart again. --- .../Item/BaseItemRepository.QueryBuilding.cs | 19 ++- .../Item/BaseItemRepository.TranslateQuery.cs | 38 +++-- .../Item/BaseItemRepositoryPlayedOrderingTests.cs | 189 +++++++++++++++++++++ 3 files changed, 228 insertions(+), 18 deletions(-) create mode 100644 tests/Jellyfin.Server.Implementations.Tests/Item/BaseItemRepositoryPlayedOrderingTests.cs diff --git a/Jellyfin.Server.Implementations/Item/BaseItemRepository.QueryBuilding.cs b/Jellyfin.Server.Implementations/Item/BaseItemRepository.QueryBuilding.cs index 05ff720ddf..c0067d8392 100644 --- a/Jellyfin.Server.Implementations/Item/BaseItemRepository.QueryBuilding.cs +++ b/Jellyfin.Server.Implementations/Item/BaseItemRepository.QueryBuilding.cs @@ -12,6 +12,7 @@ using Jellyfin.Database.Implementations; using Jellyfin.Database.Implementations.Entities; using Jellyfin.Database.Implementations.Enums; using Jellyfin.Extensions; +using Jellyfin.Server.Implementations.Extensions; using MediaBrowser.Controller.Entities; using MediaBrowser.Model.Entities; using MediaBrowser.Model.Querying; @@ -323,10 +324,21 @@ public sealed partial class BaseItemRepository orderedQuery = query.OrderBy(relevanceExpression); } + // Folders carry no played flag of their own, so these two keys go through the same predicate + // the isPlayed filter uses rather than through the stored-column lookup in OrderMapper. + Expression> MapOrderByField(ItemSortBy sortBy) => sortBy switch + { + ItemSortBy.IsPlayed when filter.User is not null + => AsOrderKey(BuildIsPlayedFilter(context, filter.User)), + ItemSortBy.IsUnplayed when filter.User is not null + => AsOrderKey(BuildIsPlayedFilter(context, filter.User).Not()), + _ => OrderMapper.MapOrderByField(sortBy, filter, context) + }; + if (orderBy.Length > 0) { var firstOrdering = orderBy[0]; - var expression = OrderMapper.MapOrderByField(firstOrdering.OrderBy, filter, context); + var expression = MapOrderByField(firstOrdering.OrderBy); if (orderedQuery is null) { @@ -350,7 +362,7 @@ public sealed partial class BaseItemRepository foreach (var item in orderBy.Skip(1)) { - expression = OrderMapper.MapOrderByField(item.OrderBy, filter, context); + expression = MapOrderByField(item.OrderBy); orderedQuery = item.SortOrder == SortOrder.Ascending ? orderedQuery.ThenBy(expression) : orderedQuery.ThenByDescending(expression); @@ -666,6 +678,9 @@ public sealed partial class BaseItemRepository return ApplyAccessFiltering(context, leafItems, new InternalItemsQuery(user) { IncludeOwnedItems = includeOwnedItems }); } + private static Expression> AsOrderKey(Expression> predicate) + => Expression.Lambda>(Expression.Convert(predicate.Body, typeof(object)), predicate.Parameters); + /// public Expression> BuildHasDescendantFilter(JellyfinDbContext context, IQueryable descendants) { diff --git a/Jellyfin.Server.Implementations/Item/BaseItemRepository.TranslateQuery.cs b/Jellyfin.Server.Implementations/Item/BaseItemRepository.TranslateQuery.cs index 623c1ea0ab..1e30f0164e 100644 --- a/Jellyfin.Server.Implementations/Item/BaseItemRepository.TranslateQuery.cs +++ b/Jellyfin.Server.Implementations/Item/BaseItemRepository.TranslateQuery.cs @@ -35,6 +35,27 @@ public sealed partial class BaseItemRepository // instance across several lambdas, and this filter is combined into a tree more than once. private static Expression> IsFolderFilter => e => e.IsFolder; + // Shared by the isPlayed filter and the IsPlayed/IsUnplayed ordering so the two cannot disagree. + private Expression> BuildIsPlayedFilter(JellyfinDbContext context, User user) + { + var userId = user.Id; + + // Leaf items carry their own played state. + var playedItemIds = context.UserData + .Where(ud => ud.UserId == userId && ud.Played) + .Select(ud => ud.ItemId); + + // Folders (Series, Seasons, BoxSets, albums, ...) have none and count as played once no + // descendant is left unplayed, matching what the DTO reports for them. This has to key off + // the item itself rather than off the requested item types: tag and collection listings mix + // folders and leaf items in a single query. + var unplayedLeafItems = GetAccessFilteredLeafItemsQuery(context, user) + .Where(e => !e.UserData!.Any(ud => ud.UserId == userId && ud.Played)); + + return IsFolderFilter.And(BuildHasDescendantFilter(context, unplayedLeafItems).Not()) + .Or(IsFolderFilter.Not().And(e => playedItemIds.Contains(e.Id))); + } + // "und" is the language filters' stand-in for a track that declares no language at all. private static string NormalizeLanguage(string language) => string.Equals(language, "und", StringComparison.OrdinalIgnoreCase) ? "und" : language; @@ -523,22 +544,7 @@ public sealed partial class BaseItemRepository if (filter.IsPlayed.HasValue) { - var userId = filter.User!.Id; - - // Leaf items carry their own played state. - var playedItemIds = context.UserData - .Where(ud => ud.UserId == userId && ud.Played) - .Select(ud => ud.ItemId); - - // Folders (Series, Seasons, BoxSets, albums, ...) have none and count as played once no - // descendant is left unplayed, matching what the DTO reports for them. This has to key off - // the item itself rather than off the requested item types: tag and collection listings mix - // folders and leaf items in a single query. - var unplayedLeafItems = GetAccessFilteredLeafItemsQuery(context, filter.User!) - .Where(e => !e.UserData!.Any(ud => ud.UserId == userId && ud.Played)); - - var isPlayedFilter = IsFolderFilter.And(BuildHasDescendantFilter(context, unplayedLeafItems).Not()) - .Or(IsFolderFilter.Not().And(e => playedItemIds.Contains(e.Id))); + var isPlayedFilter = BuildIsPlayedFilter(context, filter.User!); baseQuery = baseQuery.Where(filter.IsPlayed.Value ? isPlayedFilter : isPlayedFilter.Not()); } diff --git a/tests/Jellyfin.Server.Implementations.Tests/Item/BaseItemRepositoryPlayedOrderingTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Item/BaseItemRepositoryPlayedOrderingTests.cs new file mode 100644 index 0000000000..c2518f13b2 --- /dev/null +++ b/tests/Jellyfin.Server.Implementations.Tests/Item/BaseItemRepositoryPlayedOrderingTests.cs @@ -0,0 +1,189 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using Emby.Server.Implementations.Data; +using Jellyfin.Database.Implementations; +using Jellyfin.Database.Implementations.Entities; +using Jellyfin.Database.Implementations.Enums; +using Jellyfin.Server.Implementations.Item; +using MediaBrowser.Controller.Entities; +using Xunit; +using BaseItemKind = Jellyfin.Data.Enums.BaseItemKind; +using ItemSortBy = Jellyfin.Data.Enums.ItemSortBy; +using LinkedChildType = Jellyfin.Database.Implementations.Entities.LinkedChildType; + +namespace Jellyfin.Server.Implementations.Tests.Item; + +/// +/// Covers ordering by and , which +/// has to read the played state the isPlayed filter reports: folders hold none of their own and count +/// as played once no descendant is left unplayed. +/// +public sealed class BaseItemRepositoryPlayedOrderingTests : SqliteDbTestFixture +{ + private const string SeriesType = "MediaBrowser.Controller.Entities.TV.Series"; + private const string EpisodeType = "MediaBrowser.Controller.Entities.TV.Episode"; + private const string BoxSetType = "MediaBrowser.Controller.Entities.Movies.BoxSet"; + private const string MovieType = "MediaBrowser.Controller.Entities.Movies.Movie"; + + private readonly BaseItemRepository _repository; + private readonly User _user = new("test", "auth-provider", "reset-provider"); + + // Names run A..F so name order interleaves the two groups: a dropped or inverted played key shows + // up as a different sequence rather than as the expected one by luck. + private readonly Guid _watchedSeries = Guid.NewGuid(); + private readonly Guid _unwatchedSeries = Guid.NewGuid(); + private readonly Guid _partiallyWatchedSeries = Guid.NewGuid(); + private readonly Guid _secondUnwatchedSeries = Guid.NewGuid(); + private readonly Guid _thirdUnwatchedSeries = Guid.NewGuid(); + private readonly Guid _secondWatchedSeries = Guid.NewGuid(); + + // Box sets reach their children through LinkedChildren instead of the ancestor chain. + private readonly Guid _watchedBoxSet = Guid.NewGuid(); + private readonly Guid _unwatchedBoxSet = Guid.NewGuid(); + + private readonly HashSet _unwatchedSeriesIds; + + public BaseItemRepositoryPlayedOrderingTests() + { + _unwatchedSeriesIds = [_unwatchedSeries, _partiallyWatchedSeries, _secondUnwatchedSeries, _thirdUnwatchedSeries]; + + using (var context = CreateDbContext()) + { + Seed(context); + } + + _repository = CreateBaseItemRepository(new ItemTypeLookup()); + } + + [Fact] + public void IsPlayed_OrdersUnwatchedSeriesBeforeWatchedOnes() + { + Assert.Equal( + [_unwatchedSeries, _partiallyWatchedSeries, _secondUnwatchedSeries, _thirdUnwatchedSeries, _watchedSeries, _secondWatchedSeries], + SeriesIds(ItemSortBy.IsPlayed)); + } + + [Fact] + public void IsPlayed_CountsAPartiallyWatchedSeriesAsUnwatched() + { + var ids = SeriesIds(ItemSortBy.IsPlayed); + + Assert.True(ids.IndexOf(_partiallyWatchedSeries) < ids.IndexOf(_watchedSeries)); + } + + [Fact] + public void IsUnplayed_ReversesTheGroups() + { + Assert.Equal( + [_watchedSeries, _secondWatchedSeries, _unwatchedSeries, _partiallyWatchedSeries, _secondUnwatchedSeries, _thirdUnwatchedSeries], + SeriesIds(ItemSortBy.IsUnplayed)); + } + + [Fact] + public void IsPlayed_OrdersAnUnwatchedBoxSetBeforeAWatchedOne() + { + var ids = _repository + .GetItemList(Query(BaseItemKind.BoxSet, (ItemSortBy.IsPlayed, SortOrder.Ascending))) + .Select(i => i.Id); + + Assert.Equal([_unwatchedBoxSet, _watchedBoxSet], ids); + } + + [Fact] + public void IsPlayedThenRandom_StillPlacesEveryUnwatchedSeriesFirst() + { + var order = _repository + .GetItemList(Query(BaseItemKind.Series, (ItemSortBy.IsPlayed, SortOrder.Ascending), (ItemSortBy.Random, SortOrder.Ascending))) + .Select(i => i.Id); + + Assert.Equal(_unwatchedSeriesIds, order.Take(_unwatchedSeriesIds.Count).ToHashSet()); + } + + [Fact] + public void IsPlayedThenRandom_FillsAPageWithUnwatchedSeries() + { + var page = _repository.GetItems(new InternalItemsQuery(_user) + { + IncludeItemTypes = [BaseItemKind.Series], + OrderBy = [(ItemSortBy.IsPlayed, SortOrder.Ascending), (ItemSortBy.Random, SortOrder.Ascending)], + Limit = 4, + EnableTotalRecordCount = true + }); + + Assert.Equal(6, page.TotalRecordCount); + Assert.Equal(_unwatchedSeriesIds, page.Items.Select(i => i.Id).ToHashSet()); + } + + private List SeriesIds(ItemSortBy sortBy) + => _repository + .GetItemList(Query(BaseItemKind.Series, (sortBy, SortOrder.Ascending))) + .Select(i => i.Id) + .ToList(); + + private InternalItemsQuery Query(BaseItemKind kind, params (ItemSortBy OrderBy, SortOrder SortOrder)[] orderBy) + => new(_user) + { + IncludeItemTypes = [kind], + OrderBy = orderBy + }; + + private void Seed(JellyfinDbContext context) + { + context.Users.Add(_user); + + AddSeries(context, _watchedSeries, "A watched", playedEpisodes: 1, unplayedEpisodes: 0); + AddSeries(context, _unwatchedSeries, "B unwatched", playedEpisodes: 0, unplayedEpisodes: 1); + AddSeries(context, _partiallyWatchedSeries, "C partially watched", playedEpisodes: 1, unplayedEpisodes: 1); + AddSeries(context, _secondUnwatchedSeries, "D unwatched", playedEpisodes: 0, unplayedEpisodes: 1); + AddSeries(context, _thirdUnwatchedSeries, "E unwatched", playedEpisodes: 0, unplayedEpisodes: 1); + AddSeries(context, _secondWatchedSeries, "F watched", playedEpisodes: 1, unplayedEpisodes: 0); + + AddBoxSet(context, _watchedBoxSet, "A watched set", played: true); + AddBoxSet(context, _unwatchedBoxSet, "B unwatched set", played: false); + + context.SaveChanges(); + } + + private void AddSeries(JellyfinDbContext context, Guid id, string name, int playedEpisodes, int unplayedEpisodes) + { + context.BaseItems.Add(new BaseItemEntity { Id = id, Type = SeriesType, Name = name, SortName = name, PresentationUniqueKey = id.ToString("N"), IsFolder = true }); + + for (var i = 0; i < playedEpisodes + unplayedEpisodes; i++) + { + var episodeId = Guid.NewGuid(); + context.BaseItems.Add(new BaseItemEntity { Id = episodeId, Type = EpisodeType, Name = $"{name} {i}", PresentationUniqueKey = episodeId.ToString("N"), SeriesId = id }); + context.AncestorIds.Add(new AncestorId { ItemId = episodeId, ParentItemId = id, Item = null!, ParentItem = null! }); + + if (i < playedEpisodes) + { + AddPlayedUserData(context, episodeId); + } + } + } + + private void AddBoxSet(JellyfinDbContext context, Guid id, string name, bool played) + { + var movieId = Guid.NewGuid(); + + context.BaseItems.Add(new BaseItemEntity { Id = id, Type = BoxSetType, Name = name, SortName = name, PresentationUniqueKey = id.ToString("N"), IsFolder = true }); + context.BaseItems.Add(new BaseItemEntity { Id = movieId, Type = MovieType, Name = $"{name} movie", PresentationUniqueKey = movieId.ToString("N") }); + context.LinkedChildren.Add(new LinkedChildEntity { ParentId = id, ChildId = movieId, ChildType = LinkedChildType.Manual, SortOrder = 0 }); + + if (played) + { + AddPlayedUserData(context, movieId); + } + } + + private void AddPlayedUserData(JellyfinDbContext context, Guid itemId) + => context.UserData.Add(new UserData + { + ItemId = itemId, + UserId = _user.Id, + CustomDataKey = itemId.ToString("N"), + Played = true, + Item = null!, + User = null! + }); +} -- cgit v1.2.3