diff options
3 files changed, 228 insertions, 18 deletions
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<Func<BaseItemEntity, object?>> 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<Func<BaseItemEntity, object?>> AsOrderKey(Expression<Func<BaseItemEntity, bool>> predicate) + => Expression.Lambda<Func<BaseItemEntity, object?>>(Expression.Convert(predicate.Body, typeof(object)), predicate.Parameters); + /// <inheritdoc /> public Expression<Func<BaseItemEntity, bool>> BuildHasDescendantFilter(JellyfinDbContext context, IQueryable<BaseItemEntity> 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<Func<BaseItemEntity, bool>> IsFolderFilter => e => e.IsFolder; + // Shared by the isPlayed filter and the IsPlayed/IsUnplayed ordering so the two cannot disagree. + private Expression<Func<BaseItemEntity, bool>> 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; + +/// <summary> +/// Covers ordering by <see cref="ItemSortBy.IsPlayed"/> and <see cref="ItemSortBy.IsUnplayed"/>, 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. +/// </summary> +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<Guid> _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<Guid> 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! + }); +} |