diff options
16 files changed, 1226 insertions, 28 deletions
diff --git a/Emby.Server.Implementations/Library/Search/SearchManager.cs b/Emby.Server.Implementations/Library/Search/SearchManager.cs index 306a8673d5..a8ee416b31 100644 --- a/Emby.Server.Implementations/Library/Search/SearchManager.cs +++ b/Emby.Server.Implementations/Library/Search/SearchManager.cs @@ -143,11 +143,19 @@ public class SearchManager : ISearchManager baseQuery = _queryHelpers.ApplyAccessFiltering(dbContext, baseQuery, accessFilter); - var allowedIds = await baseQuery - .Select(e => e.Id) - .ToHashSetAsync(cancellationToken) + var allowed = await baseQuery + .Select(e => new { e.Id, e.PrimaryVersionId }) + .ToListAsync(cancellationToken) .ConfigureAwait(false); + var allowedIds = allowed.Select(e => e.Id).ToHashSet(); + + // A provider can return both an alternate version and the primary it belongs to, and the + // two are one item to the user. + allowedIds.ExceptWith(allowed + .Where(e => e.PrimaryVersionId.HasValue && allowedIds.Contains(e.PrimaryVersionId.Value)) + .Select(e => e.Id)); + if (allowedIds.Count == candidates.Count) { return candidates; diff --git a/Emby.Server.Implementations/Library/Search/SqlSearchProvider.cs b/Emby.Server.Implementations/Library/Search/SqlSearchProvider.cs index c4d3b249d5..2cbfb6a4fa 100644 --- a/Emby.Server.Implementations/Library/Search/SqlSearchProvider.cs +++ b/Emby.Server.Implementations/Library/Search/SqlSearchProvider.cs @@ -115,6 +115,7 @@ public class SqlSearchProvider : IInternalSearchProvider dbQuery = ApplyMediaTypeFilter(dbQuery, query.MediaTypes); dbQuery = ApplyParentFilter(dbQuery, query.ParentId); dbQuery = ApplyUserAccessFilter(dbContext, dbQuery, query); + dbQuery = ExcludeVersionsOfMatchedPrimaries(dbQuery); // Compute the score in SQL: the ternary translates to a CASE WHEN. CleanName is // the pre-normalized (lowercase, diacritic-stripped) form, so we score against it @@ -193,6 +194,12 @@ public class SqlSearchProvider : IInternalSearchProvider return query.Where(e => e.ParentId == pid || e.Parents!.Any(p => p.ParentItemId == pid)); } + private static IQueryable<BaseItemEntity> ExcludeVersionsOfMatchedPrimaries(IQueryable<BaseItemEntity> query) + { + var matched = query; + return query.Where(e => e.PrimaryVersionId == null || !matched.Any(p => p.Id == e.PrimaryVersionId)); + } + private IQueryable<BaseItemEntity> ApplyUserAccessFilter( JellyfinDbContext dbContext, IQueryable<BaseItemEntity> query, diff --git a/Emby.Server.Implementations/Library/SimilarItems/MovieSimilarItemsProvider.cs b/Emby.Server.Implementations/Library/SimilarItems/MovieSimilarItemsProvider.cs index b1547e72fe..cc8f0fd24e 100644 --- a/Emby.Server.Implementations/Library/SimilarItems/MovieSimilarItemsProvider.cs +++ b/Emby.Server.Implementations/Library/SimilarItems/MovieSimilarItemsProvider.cs @@ -1,3 +1,5 @@ +#pragma warning disable RS0030 // Do not use banned APIs: Guid == is required inside EF expression trees. + using System; using System.Collections.Generic; using System.Linq; @@ -172,7 +174,7 @@ public sealed class MovieSimilarItemsProvider : ILocalSimilarItemsProvider<Movie var allCandidateIdsList = allCandidateIds.ToList(); var accessibleItems = await baseQuery .WhereOneOrMany(allCandidateIdsList, e => e.Id) - .Select(e => new { e.Id, e.PresentationUniqueKey }) + .Select(e => new { e.Id, e.PresentationUniqueKey, e.PrimaryVersionId }) .ToListAsync(cancellationToken).ConfigureAwait(false); // Phase 3: Pick top IDs per source, dedup by PresentationUniqueKey @@ -189,6 +191,9 @@ public sealed class MovieSimilarItemsProvider : ILocalSimilarItemsProvider<Movie var orderedIds = accessibleItems .Where(x => scores.ContainsKey(x.Id)) .OrderByDescending(x => scores.GetValueOrDefault(x.Id)) + // Two versions of one movie score the same, so name the primary as the + // representative of the group rather than whichever came back first. + .ThenBy(x => x.PrimaryVersionId.HasValue) .DistinctBy(x => x.PresentationUniqueKey) .Take(limit) .Select(x => x.Id) @@ -245,6 +250,11 @@ public sealed class MovieSimilarItemsProvider : ILocalSimilarItemsProvider<Movie result[id] = []; } + var hiddenVersionIds = context.BaseItems.AsNoTracking() + .Where(e => e.PrimaryVersionId != null + && context.BaseItems.Any(p => p.Id == e.PrimaryVersionId && p.TopParentId == e.TopParentId)) + .Select(e => e.Id); + foreach (var (valueType, weight) in _itemValueDimensions) { var sourceRows = await context.ItemValuesMap.AsNoTracking() @@ -260,7 +270,7 @@ public sealed class MovieSimilarItemsProvider : ILocalSimilarItemsProvider<Movie } var candidateRows = await context.ItemValuesMap.AsNoTracking() - .Where(m => !m.Item.PrimaryVersionId.HasValue && m.ItemValue.Type == valueType && allKeys.Contains(m.ItemValue.CleanValue)) + .Where(m => !hiddenVersionIds.Contains(m.ItemId) && m.ItemValue.Type == valueType && allKeys.Contains(m.ItemValue.CleanValue)) .Select(m => new { m.ItemId, Key = m.ItemValue.CleanValue }) .ToListAsync(cancellationToken).ConfigureAwait(false); @@ -276,7 +286,7 @@ public sealed class MovieSimilarItemsProvider : ILocalSimilarItemsProvider<Movie if (personSourceRows.Count > 0) { var personCandidateRows = await context.PeopleBaseItemMap.AsNoTracking() - .Where(m => !m.Item.PrimaryVersionId.HasValue) + .Where(m => !hiddenVersionIds.Contains(m.ItemId)) .Where(m => context.PeopleBaseItemMap .Where(s => sourceIds.Contains(s.ItemId) && _scoredPersonTypes.Contains(s.People.PersonType)) .Select(s => s.PeopleId) diff --git a/Jellyfin.Server.Implementations/Item/BaseItemRepository.QueryBuilding.cs b/Jellyfin.Server.Implementations/Item/BaseItemRepository.QueryBuilding.cs index c0067d8392..8ac6722eef 100644 --- a/Jellyfin.Server.Implementations/Item/BaseItemRepository.QueryBuilding.cs +++ b/Jellyfin.Server.Implementations/Item/BaseItemRepository.QueryBuilding.cs @@ -465,16 +465,23 @@ public sealed partial class BaseItemRepository baseQuery = ApplyParentalRestrictions(context, baseQuery, filter); - // Exclude alternate versions (have PrimaryVersionId set) and owned non-extra items. - // Extras (trailers, etc.) have OwnerId set but also have ExtraType set — keep those. + // Hide alternate versions behind the primary of their library, and exclude owned non-extra + // items. Extras (trailers, etc.) have OwnerId set but also have ExtraType set — keep those. if (!filter.IncludeOwnedItems) { - baseQuery = baseQuery.Where(e => e.PrimaryVersionId == null && (e.OwnerId == null || e.ExtraType != null)); + baseQuery = ApplyAlternateVersionFiltering(context, baseQuery) + .Where(e => e.OwnerId == null || e.ExtraType != null); } return baseQuery; } + private static IQueryable<BaseItemEntity> ApplyAlternateVersionFiltering( + JellyfinDbContext context, + IQueryable<BaseItemEntity> baseQuery) + => baseQuery.Where(e => e.PrimaryVersionId == null + || !context.BaseItems.Any(p => p.Id == e.PrimaryVersionId && p.TopParentId == e.TopParentId)); + /// <summary> /// Restricts a query to the libraries the user may open, exempting requested by-name items. /// </summary> diff --git a/Jellyfin.Server.Implementations/Item/BaseItemRepository.TranslateQuery.cs b/Jellyfin.Server.Implementations/Item/BaseItemRepository.TranslateQuery.cs index a745c3309f..486b3b5de6 100644 --- a/Jellyfin.Server.Implementations/Item/BaseItemRepository.TranslateQuery.cs +++ b/Jellyfin.Server.Implementations/Item/BaseItemRepository.TranslateQuery.cs @@ -807,11 +807,16 @@ public sealed partial class BaseItemRepository { // Exclude owned non-extra items from general queries. // Extras (trailers, etc.) have OwnerId set but also have ExtraType set - keep those. - // Alternate versions (PrimaryVersionId set) are normally excluded too, but resume queries - // keep them so the actually-played version can surface instead of collapsing onto the primary. - baseQuery = filter.IsResumable == true - ? baseQuery.Where(e => e.OwnerId == null || e.ExtraType != null) - : baseQuery.Where(e => e.PrimaryVersionId == null && (e.OwnerId == null || e.ExtraType != null)); + baseQuery = baseQuery.Where(e => e.OwnerId == null || e.ExtraType != null); + + // Alternate versions (PrimaryVersionId set) are normally hidden behind their primary, but + // resume queries keep them so the actually-played version can surface instead of collapsing + // onto the primary, and the library scan keeps them so a merged version is not mistaken for + // a new item. + if (filter.IsResumable != true && !filter.IncludeAlternateVersions) + { + baseQuery = ApplyAlternateVersionFiltering(context, baseQuery); + } } if (filter.OwnerIds.Length > 0) diff --git a/Jellyfin.Server.Implementations/Item/ItemPersistenceService.cs b/Jellyfin.Server.Implementations/Item/ItemPersistenceService.cs index c8672e189b..051b85208c 100644 --- a/Jellyfin.Server.Implementations/Item/ItemPersistenceService.cs +++ b/Jellyfin.Server.Implementations/Item/ItemPersistenceService.cs @@ -2,6 +2,7 @@ using System; using System.Collections.Generic; +using System.Globalization; using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -657,6 +658,38 @@ public class ItemPersistenceService : IItemPersistenceService sortOrder++; } + var linkedChildIds = newLinkedChildren + .Select(c => c.ChildId) + // A video listed among its own versions would be pointed at itself. + .Where(childId => existingChildIds.Contains(childId) && !childId.Equals(video.Id)) + .Where(childId => !childId.Equals(video.PrimaryVersionId)) + .ToList(); + if (linkedChildIds.Count > 0) + { + var demotedChildren = context.BaseItems + .Where(e => linkedChildIds.Contains(e.Id) + && (e.PrimaryVersionId == null || e.PrimaryVersionId != video.Id)) + .ToList(); + + foreach (var child in demotedChildren) + { + child.PrimaryVersionId = video.Id; + + // Mirrors Video.CreatePresentationUniqueKey, so presentation-key grouping + // collapses the version onto its primary as well. + child.PresentationUniqueKey = video.Id.ToString("N", CultureInfo.InvariantCulture); + } + + if (demotedChildren.Count > 0) + { + _logger.LogInformation( + "Set PrimaryVersionId on {Count} alternate versions of video {VideoName} ({VideoId})", + demotedChildren.Count, + video.Name, + video.Id); + } + } + // A previously-linked LocalAlternateVersion that is no longer present becomes orphaned; var previousLinkedChildren = allLinkedChildrenByParent.GetValueOrDefault(video.Id); if (previousLinkedChildren is { Count: > 0 }) diff --git a/Jellyfin.Server/Migrations/Routines/20260908120000_RepairAlternateVersionLinks.cs b/Jellyfin.Server/Migrations/Routines/20260908120000_RepairAlternateVersionLinks.cs new file mode 100644 index 0000000000..43db38c92f --- /dev/null +++ b/Jellyfin.Server/Migrations/Routines/20260908120000_RepairAlternateVersionLinks.cs @@ -0,0 +1,216 @@ +using System; +using System.Collections.Generic; +using System.Globalization; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Jellyfin.Database.Implementations; +using Jellyfin.Server.ServerSetupApp; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Logging; +using LinkedChildType = Jellyfin.Database.Implementations.Entities.LinkedChildType; + +namespace Jellyfin.Server.Migrations.Routines; + +/// <summary> +/// Re-points every video that is linked as an alternate version at the primary it belongs to. +/// </summary> +[JellyfinMigration("2026-09-08T12:00:00", nameof(RepairAlternateVersionLinks))] +[JellyfinMigrationBackup(JellyfinDb = true)] +internal class RepairAlternateVersionLinks : IAsyncMigrationRoutine +{ + private const int BatchSize = 1000; + + private readonly IStartupLogger<RepairAlternateVersionLinks> _logger; + private readonly IDbContextFactory<JellyfinDbContext> _dbProvider; + + /// <summary> + /// Initializes a new instance of the <see cref="RepairAlternateVersionLinks"/> class. + /// </summary> + /// <param name="logger">The startup logger.</param> + /// <param name="dbProvider">The database context factory.</param> + public RepairAlternateVersionLinks( + IStartupLogger<RepairAlternateVersionLinks> logger, + IDbContextFactory<JellyfinDbContext> dbProvider) + { + _logger = logger; + _dbProvider = dbProvider; + } + + /// <inheritdoc /> + public async Task PerformAsync(CancellationToken cancellationToken) + { + var dbContext = await _dbProvider.CreateDbContextAsync(cancellationToken).ConfigureAwait(false); + await using (dbContext.ConfigureAwait(false)) + { + var links = await dbContext.LinkedChildren + .Where(lc => lc.ChildType == LinkedChildType.LocalAlternateVersion + || lc.ChildType == LinkedChildType.LinkedAlternateVersion) + .Select(lc => new { lc.ParentId, lc.ChildId, lc.ChildType }) + .ToListAsync(cancellationToken) + .ConfigureAwait(false); + + if (links.Count == 0) + { + _logger.LogInformation("No alternate version links found, nothing to repair."); + return; + } + + // A version belongs to one primary; a file-based link outranks a user-merged one, as it + // does everywhere else these two link types meet. + var primaryByChild = links + .GroupBy(l => l.ChildId) + .ToDictionary( + g => g.Key, + g => g.OrderBy(l => l.ChildType == LinkedChildType.LocalAlternateVersion ? 0 : 1) + .First() + .ParentId); + + // A version that is its own primary would be hidden from every list by the repair below. + foreach (var selfLink in primaryByChild.Where(kvp => kvp.Value.Equals(kvp.Key)).ToList()) + { + _logger.LogWarning("Skipping alternate version {ChildId}, which is linked to itself.", selfLink.Key); + primaryByChild.Remove(selfLink.Key); + } + + ResolvePrimaries(primaryByChild); + + var repaired = 0; + var promoted = 0; + + // The primaries are loaded along with their versions: a primary that carries a + // PrimaryVersionId of its own hides the whole group it heads. + var itemIds = primaryByChild.Keys.Concat(primaryByChild.Values).Distinct().ToList(); + for (var offset = 0; offset < itemIds.Count; offset += BatchSize) + { + cancellationToken.ThrowIfCancellationRequested(); + + var batch = itemIds.GetRange(offset, Math.Min(BatchSize, itemIds.Count - offset)); + var items = await dbContext.BaseItems + .Where(e => batch.Contains(e.Id)) + .ToListAsync(cancellationToken) + .ConfigureAwait(false); + + foreach (var item in items) + { + if (primaryByChild.TryGetValue(item.Id, out var primaryId)) + { + // Mirrors Video.CreatePresentationUniqueKey for a video that has a primary. + var expectedKey = primaryId.ToString("N", CultureInfo.InvariantCulture); + if (item.PrimaryVersionId.HasValue + && primaryId.Equals(item.PrimaryVersionId.Value) + && string.Equals(item.PresentationUniqueKey, expectedKey, StringComparison.Ordinal)) + { + continue; + } + + item.PrimaryVersionId = primaryId; + item.PresentationUniqueKey = expectedKey; + repaired++; + } + else if (item.PrimaryVersionId.HasValue) + { + if (item.OwnerId.HasValue) + { + // An owned item is hidden by its owner rather than by its primary, so + // clearing the primary here would not bring the group back. + _logger.LogWarning( + "Alternate versions are linked to {ItemId}, which is owned by {OwnerId}; the group stays hidden until the owner is repaired.", + item.Id, + item.OwnerId.Value); + continue; + } + + // Nothing links this one as a version, so the leftover primary is stale and + // would hide it, and with it every version linked to it. + _logger.LogWarning( + "Clearing the stale primary {PrimaryVersionId} of {ItemId}, which other versions are linked to.", + item.PrimaryVersionId.Value, + item.Id); + + item.PrimaryVersionId = null; + item.PresentationUniqueKey = item.Id.ToString("N", CultureInfo.InvariantCulture); + promoted++; + } + } + + await dbContext.SaveChangesAsync(cancellationToken).ConfigureAwait(false); + } + + _logger.LogInformation( + "Repaired {Repaired} of {Total} alternate version links, and promoted {Promoted} primaries that were versions themselves.", + repaired, + primaryByChild.Count, + promoted); + } + } + + private void ResolvePrimaries(Dictionary<Guid, Guid> primaryByChild) + { + var resolvedPrimaries = new Dictionary<Guid, Guid>(primaryByChild.Count); + + foreach (var start in primaryByChild.Keys.ToList()) + { + if (resolvedPrimaries.ContainsKey(start)) + { + continue; + } + + var chain = new List<Guid>(); + var walked = new HashSet<Guid>(); + var current = start; + Guid primary; + + while (true) + { + if (resolvedPrimaries.TryGetValue(current, out var resolved)) + { + primary = resolved; + break; + } + + if (!primaryByChild.TryGetValue(current, out var next)) + { + // Nothing is linking this one as a version of something else, so it heads the group. + primary = current; + break; + } + + if (!walked.Add(current)) + { + var loop = chain.Skip(chain.IndexOf(current)).ToList(); + + // Which member heads the group is arbitrary; the lowest id keeps the repair + // stable if the migration is ever re-run over the same data. + primary = loop.Min(); + _logger.LogWarning( + "Alternate version links form a loop ({Loop}); keeping {PrimaryId} as the primary of the group.", + string.Join(" -> ", loop), + primary); + + primaryByChild.Remove(primary); + break; + } + + chain.Add(current); + current = next; + } + + foreach (var version in chain) + { + resolvedPrimaries[version] = primary; + } + } + + foreach (var (version, primary) in resolvedPrimaries) + { + if (primary.Equals(version)) + { + // The member of a loop that was kept as the primary of its group. + continue; + } + + primaryByChild[version] = primary; + } + } +} diff --git a/MediaBrowser.Controller/Entities/Folder.cs b/MediaBrowser.Controller/Entities/Folder.cs index d8203ea6f2..b3e7369205 100644 --- a/MediaBrowser.Controller/Entities/Folder.cs +++ b/MediaBrowser.Controller/Entities/Folder.cs @@ -316,7 +316,7 @@ namespace MediaBrowser.Controller.Entities var dictionary = new Dictionary<Guid, BaseItem>(); Children = null; // invalidate cached children. - var childrenList = Children.ToList(); + var childrenList = GetChildrenForValidation(); foreach (var child in childrenList) { @@ -551,7 +551,7 @@ namespace MediaBrowser.Controller.Entities && primaryVideo.OwnerId.IsEmpty() && (primaryVideo.LocalAlternateVersions ?? []).Any(p => alternateVersionPaths.Contains(p))) { - var newPrimary = newItems + var newPrimary = validChildren .OfType<Video>() .FirstOrDefault(v => (v.LocalAlternateVersions ?? []) .Any(p => (primaryVideo.LocalAlternateVersions ?? []) @@ -593,6 +593,8 @@ namespace MediaBrowser.Controller.Entities newPrimary.Name, newPrimary.Id); + await PromoteToPrimaryVersionAsync(newPrimary, cancellationToken).ConfigureAwait(false); + // Reroute collection/playlist references from old primary to new primary await LibraryManager.RerouteLinkedChildReferencesAsync(oldPrimary.Id, newPrimary.Id).ConfigureAwait(false); @@ -621,9 +623,12 @@ namespace MediaBrowser.Controller.Entities LibraryManager.DeleteItem(oldPrimary, new DeleteOptions { DeleteFileLocation = false }, this, false); } - // Demote old primaries that are now alternate versions of newly created primaries. + // Demote old primaries that are now alternate versions of another primary. // This handles the case where a new file is added that becomes the new primary - // (e.g. movie-2 added, movie-3 was primary → movie-3 needs demotion). + // (e.g. movie-2 added, movie-3 was primary → movie-3 needs demotion), and the case + // where the file that takes over was already in the library and merely traded + // places with this one — so the new primary is looked up among all valid children + // rather than only the newly created ones. // Items in replacedPrimaries are excluded (already in actuallyRemoved). var oldPrimariesToDemote = new List<(Video OldPrimary, Video NewPrimary)>(); foreach (var item in itemsRemoved.Except(actuallyRemoved)) @@ -633,7 +638,7 @@ namespace MediaBrowser.Controller.Entities && !string.IsNullOrEmpty(item.Path) && alternateVersionPaths.Contains(item.Path)) { - var newPrimary = newItems + var newPrimary = validChildren .OfType<Video>() .FirstOrDefault(v => (v.LocalAlternateVersions ?? []) .Any(p => string.Equals(p, item.Path, StringComparison.OrdinalIgnoreCase))); @@ -653,10 +658,13 @@ namespace MediaBrowser.Controller.Entities newPrimary.Name, newPrimary.Id); + await PromoteToPrimaryVersionAsync(newPrimary, cancellationToken).ConfigureAwait(false); + // First: update old primary's alternate items to point to new primary. // Order matters — update alternates FIRST so they don't get orphan-deleted // when old primary's arrays are cleared. - var oldAlternateIds = LibraryManager.GetLocalAlternateVersionIds(oldPrimary) + var oldLocalAlternateIds = LibraryManager.GetLocalAlternateVersionIds(oldPrimary).ToHashSet(); + var oldAlternateIds = oldLocalAlternateIds .Concat(LibraryManager.GetLinkedAlternateVersions(oldPrimary).Select(v => v.Id)) .Distinct() .ToList(); @@ -666,7 +674,10 @@ namespace MediaBrowser.Controller.Entities if (LibraryManager.GetItemById(altId) is Video altVideo && !altVideo.Id.Equals(newPrimary.Id)) { altVideo.SetPrimaryVersionId(newPrimary.Id); - altVideo.OwnerId = newPrimary.Id; + + // Only a version stored next to the new primary is owned by it; one that + // was merged in by hand keeps its own row and must stay unowned. + altVideo.OwnerId = oldLocalAlternateIds.Contains(altVideo.Id) ? newPrimary.Id : Guid.Empty; await altVideo.UpdateToRepositoryAsync(ItemUpdateType.MetadataEdit, cancellationToken).ConfigureAwait(false); } } @@ -772,6 +783,23 @@ namespace MediaBrowser.Controller.Entities } } + private async Task PromoteToPrimaryVersionAsync(Video newPrimary, CancellationToken cancellationToken) + { + if (!newPrimary.PrimaryVersionId.HasValue && newPrimary.OwnerId.IsEmpty()) + { + return; + } + + Logger.LogInformation( + "Promoting {Name} ({Id}) to the primary version of its group", + newPrimary.Name, + newPrimary.Id); + + newPrimary.SetPrimaryVersionId(null); + newPrimary.OwnerId = Guid.Empty; + await newPrimary.UpdateToRepositoryAsync(ItemUpdateType.MetadataEdit, cancellationToken).ConfigureAwait(false); + } + private async Task RefreshMetadataRecursive(IList<BaseItem> children, MetadataRefreshOptions refreshOptions, bool recursive, IProgress<double> progress, CancellationToken cancellationToken) { await RunTasks( @@ -876,6 +904,17 @@ namespace MediaBrowser.Controller.Entities }); } + private IReadOnlyList<BaseItem> GetChildrenForValidation() + { + return ItemRepository.GetItemList(new InternalItemsQuery + { + Parent = this, + GroupByPresentationUniqueKey = false, + IncludeAlternateVersions = true, + DtoOptions = new DtoOptions(true) + }); + } + public virtual int GetChildCount(User user) { if (LinkedChildren.Length > 0) diff --git a/MediaBrowser.Controller/Entities/InternalItemsQuery.cs b/MediaBrowser.Controller/Entities/InternalItemsQuery.cs index eb2a3676ac..7c88d5dd05 100644 --- a/MediaBrowser.Controller/Entities/InternalItemsQuery.cs +++ b/MediaBrowser.Controller/Entities/InternalItemsQuery.cs @@ -488,6 +488,14 @@ namespace MediaBrowser.Controller.Entities /// </summary> public bool IncludeOwnedItems { get; set; } + /// <summary> + /// Gets or sets a value indicating whether to include alternate versions, which carry a + /// <see cref="Video.PrimaryVersionId"/> and are normally hidden behind the version they + /// belong to. Unlike <see cref="IncludeOwnedItems"/> this keeps the versions a user merged + /// by hand without also returning the parts and extras owned by another item. + /// </summary> + public bool IncludeAlternateVersions { get; set; } + public bool? Is4K { get; set; } public int? MaxHeight { get; set; } diff --git a/tests/Jellyfin.Controller.Tests/Entities/BaseItemTests.cs b/tests/Jellyfin.Controller.Tests/Entities/BaseItemTests.cs index 86bac4256a..f9b29e576b 100644 --- a/tests/Jellyfin.Controller.Tests/Entities/BaseItemTests.cs +++ b/tests/Jellyfin.Controller.Tests/Entities/BaseItemTests.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Globalization; using System.IO; using System.Linq; using System.Reflection; @@ -29,6 +30,23 @@ namespace Jellyfin.Controller.Tests.Entities; public class BaseItemTests { [Fact] + public void SetPrimaryVersionId_Null_RestoresTheItemsOwnPresentationKey() + { + var primaryId = Guid.NewGuid(); + var video = new Video { Id = Guid.NewGuid(), Path = "/Movies/Movie/Movie - 4K.mkv" }; + + // While it is a version, it presents as the primary so lists collapse the two together. + video.SetPrimaryVersionId(primaryId); + Assert.Equal(primaryId.ToString("N", CultureInfo.InvariantCulture), video.PresentationUniqueKey); + + // Promoting it back has to restore its own key, or it keeps collapsing onto - and staying + // hidden behind - a primary it no longer belongs to. + video.SetPrimaryVersionId(null); + Assert.Null(video.PrimaryVersionId); + Assert.Equal(video.Id.ToString("N", CultureInfo.InvariantCulture), video.PresentationUniqueKey); + } + + [Fact] public void GetItemByNameFolderName_ShortName_IsKeptAsIs() { SetupPassThroughFileSystem(); diff --git a/tests/Jellyfin.Server.Implementations.Tests/Item/BaseItemRepositoryChildrenTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Item/BaseItemRepositoryChildrenTests.cs new file mode 100644 index 0000000000..5e045e9f83 --- /dev/null +++ b/tests/Jellyfin.Server.Implementations.Tests/Item/BaseItemRepositoryChildrenTests.cs @@ -0,0 +1,103 @@ +using System; +using System.Linq; +using Emby.Server.Implementations.Data; +using Jellyfin.Database.Implementations.Entities; +using Jellyfin.Server.Implementations.Item; +using MediaBrowser.Controller.Entities; +using Xunit; +using BaseItemKind = Jellyfin.Data.Enums.BaseItemKind; + +namespace Jellyfin.Server.Implementations.Tests.Item; + +/// <summary> +/// Covers the children query the library scan runs against a folder: a version merged by hand is +/// hidden from ordinary queries, but the scan has to see it or it takes the row for a new item and +/// recreates it, splitting the version group apart again. +/// </summary> +public sealed class BaseItemRepositoryChildrenTests : SqliteDbTestFixture +{ + private static readonly Guid _folderId = Guid.Parse("aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"); + private static readonly Guid _primaryId = Guid.Parse("bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb"); + private static readonly Guid _mergedVersionId = Guid.Parse("cccccccc-cccc-cccc-cccc-cccccccccccc"); + private static readonly Guid _ownedVersionId = Guid.Parse("dddddddd-dddd-dddd-dddd-dddddddddddd"); + + private readonly BaseItemRepository _repository; + + public BaseItemRepositoryChildrenTests() + { + var itemTypeLookup = new ItemTypeLookup(); + _repository = CreateBaseItemRepository(itemTypeLookup); + + var movieTypeName = itemTypeLookup.BaseItemKindNames[BaseItemKind.Movie]; + using var ctx = CreateDbContext(); + ctx.BaseItems.Add(new BaseItemEntity + { + Id = _folderId, + Type = itemTypeLookup.BaseItemKindNames[BaseItemKind.Folder]!, + Name = "Movies", + Path = "/movies", + IsFolder = true + }); + ctx.BaseItems.Add(CreateMovie(_primaryId, movieTypeName!, "Big Buck Bunny", "/media1/Big Buck Bunny/bbb-1080p.mp4", null, null)); + ctx.BaseItems.Add(CreateMovie(_mergedVersionId, movieTypeName!, "Big Buck Bunny", "/media2/Big Buck Bunny/bbb-2160p.mp4", _primaryId, null)); + ctx.BaseItems.Add(CreateMovie(_ownedVersionId, movieTypeName!, "Big Buck Bunny - 720p", "/media1/Big Buck Bunny/bbb-720p.mp4", _primaryId, _primaryId)); + ctx.SaveChanges(); + } + + [Fact] + public void GetItemList_ChildrenOfFolder_ExcludesAlternateVersionsByDefault() + { + var result = _repository.GetItemList(new InternalItemsQuery { ParentId = _folderId }); + + var item = Assert.Single(result); + Assert.Equal(_primaryId, item.Id); + } + + [Fact] + public void GetItemList_ChildrenOfFolderIncludingAlternateVersions_KeepsMergedVersion() + { + var result = _repository.GetItemList(new InternalItemsQuery + { + ParentId = _folderId, + IncludeAlternateVersions = true + }); + + Assert.Equal(2, result.Count); + Assert.Contains(result, i => i.Id.Equals(_primaryId)); + Assert.Contains(result, i => i.Id.Equals(_mergedVersionId)); + } + + [Fact] + public void GetItemList_ChildrenOfFolderIncludingAlternateVersions_StillExcludesOwnedVersion() + { + // A version stored next to the file it belongs to is owned by its primary and is never + // resolved on its own, so the scan must not see it as a child of the folder either. + var result = _repository.GetItemList(new InternalItemsQuery + { + ParentId = _folderId, + IncludeAlternateVersions = true + }); + + Assert.DoesNotContain(result, i => i.Id.Equals(_ownedVersionId)); + } + + private static BaseItemEntity CreateMovie(Guid id, string typeName, string name, string path, Guid? primaryVersionId, Guid? ownerId) + { + return new BaseItemEntity + { + Id = id, + Type = typeName, + Name = name, + Path = path, + ParentId = _folderId, + TopParentId = _folderId, + PresentationUniqueKey = (primaryVersionId ?? id).ToString("N"), + PrimaryVersionId = primaryVersionId, + OwnerId = ownerId, + MediaType = "Video", + IsMovie = true, + IsFolder = false, + IsVirtualItem = false + }; + } +} diff --git a/tests/Jellyfin.Server.Implementations.Tests/Item/BaseItemRepositoryGroupingTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Item/BaseItemRepositoryGroupingTests.cs index 535961a66c..9238ec9fd1 100644 --- a/tests/Jellyfin.Server.Implementations.Tests/Item/BaseItemRepositoryGroupingTests.cs +++ b/tests/Jellyfin.Server.Implementations.Tests/Item/BaseItemRepositoryGroupingTests.cs @@ -13,13 +13,18 @@ namespace Jellyfin.Server.Implementations.Tests.Item; public sealed class BaseItemRepositoryGroupingTests : SqliteDbTestFixture { + private static readonly Guid _movieLibraryId = Guid.Parse("aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"); + private static readonly Guid _movie4KLibraryId = Guid.Parse("bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb"); + private readonly BaseItemRepository _repository; private readonly string _movieTypeName; + private readonly string _folderTypeName; public BaseItemRepositoryGroupingTests() { var itemTypeLookup = new ItemTypeLookup(); _movieTypeName = itemTypeLookup.BaseItemKindNames[BaseItemKind.Movie]; + _folderTypeName = itemTypeLookup.BaseItemKindNames[BaseItemKind.Folder]; _repository = CreateBaseItemRepository(itemTypeLookup); } @@ -67,6 +72,118 @@ public sealed class BaseItemRepositoryGroupingTests : SqliteDbTestFixture Assert.Equal(firstId, item.Id); } + [Fact] + public void GetItemList_LibraryWithoutThePrimaryOfTheGroup_KeepsTheVersionVisible() + { + var primaryId = Guid.Parse("33333333-3333-3333-3333-333333333333"); + var versionId = Guid.Parse("44444444-4444-4444-4444-444444444444"); + var sameLibraryPrimaryId = Guid.Parse("55555555-5555-5555-5555-555555555555"); + var sameLibraryVersionId = Guid.Parse("66666666-6666-6666-6666-666666666666"); + + SeedCrossLibraryGroup(primaryId, versionId, sameLibraryPrimaryId, sameLibraryVersionId); + + var result = _repository.GetItemList(CreateLibraryQuery(_movieLibraryId)); + + // The version stands in for the group in the library it lives in, because its primary is in + // a library of its own; a group merged inside this library still collapses onto its primary. + Assert.Contains(result, i => i.Id.Equals(versionId)); + Assert.Contains(result, i => i.Id.Equals(sameLibraryPrimaryId)); + Assert.DoesNotContain(result, i => i.Id.Equals(sameLibraryVersionId)); + Assert.DoesNotContain(result, i => i.Id.Equals(primaryId)); + } + + [Fact] + public void GetItemList_LibraryHoldingThePrimary_ReturnsThePrimary() + { + var primaryId = Guid.Parse("33333333-3333-3333-3333-333333333333"); + var versionId = Guid.Parse("44444444-4444-4444-4444-444444444444"); + + SeedCrossLibraryGroup(primaryId, versionId); + + var result = _repository.GetItemList(CreateLibraryQuery(_movie4KLibraryId)); + + var item = Assert.Single(result); + Assert.Equal(primaryId, item.Id); + } + + [Fact] + public void GetItemList_BothLibrariesOfACrossLibraryGroup_ReturnsItOnce() + { + var primaryId = Guid.Parse("33333333-3333-3333-3333-333333333333"); + var versionId = Guid.Parse("44444444-4444-4444-4444-444444444444"); + + SeedCrossLibraryGroup(primaryId, versionId); + + var result = _repository.GetItemList(CreateLibraryQuery(_movieLibraryId, _movie4KLibraryId)); + + // With both libraries in scope the presentation key grouping collapses the version. + var item = Assert.Single(result); + Assert.Equal(primaryId, item.Id); + } + + [Fact] + public void GetItems_LibraryWithoutThePrimaryOfTheGroup_CountsWhatItLists() + { + var primaryId = Guid.Parse("33333333-3333-3333-3333-333333333333"); + var versionId = Guid.Parse("44444444-4444-4444-4444-444444444444"); + + SeedCrossLibraryGroup(primaryId, versionId); + + var listed = _repository.GetItemList(CreateLibraryQuery(_movieLibraryId)).Count; + + var query = CreateLibraryQuery(_movieLibraryId); + query.EnableTotalRecordCount = true; + query.Limit = 1; + + // The total the client pages against has to agree with the listing. + Assert.Equal(1, listed); + Assert.Equal(listed, _repository.GetItems(query).TotalRecordCount); + } + + private static InternalItemsQuery CreateLibraryQuery(params Guid[] topParentIds) + { + return new InternalItemsQuery(new Database.Implementations.Entities.User("test", "auth", "reset")) + { + IncludeItemTypes = [BaseItemKind.Movie], + TopParentIds = topParentIds + }; + } + + private void SeedCrossLibraryGroup( + Guid primaryId, + Guid versionId, + Guid? sameLibraryPrimaryId = null, + Guid? sameLibraryVersionId = null) + { + using var ctx = CreateDbContext(); + ctx.BaseItems.Add(CreateFolderEntity(_movieLibraryId, "Movies")); + ctx.BaseItems.Add(CreateFolderEntity(_movie4KLibraryId, "Movies-4K")); + + // The 4K version heads the group and lives in a library of its own. + ctx.BaseItems.Add(CreateMovieEntity(primaryId, "Movie - 4K", primaryId.ToString("N"), null, _movie4KLibraryId)); + ctx.BaseItems.Add(CreateMovieEntity(versionId, "Movie", primaryId.ToString("N"), primaryId, _movieLibraryId)); + + if (sameLibraryPrimaryId.HasValue && sameLibraryVersionId.HasValue) + { + ctx.BaseItems.Add(CreateMovieEntity(sameLibraryPrimaryId.Value, "Other - 4K", sameLibraryPrimaryId.Value.ToString("N"), null, _movieLibraryId)); + ctx.BaseItems.Add(CreateMovieEntity(sameLibraryVersionId.Value, "Other", sameLibraryPrimaryId.Value.ToString("N"), sameLibraryPrimaryId.Value, _movieLibraryId)); + } + + ctx.SaveChanges(); + } + + private BaseItemEntity CreateFolderEntity(Guid id, string name) + { + return new BaseItemEntity + { + Id = id, + Type = _folderTypeName, + Name = name, + Path = "/" + name, + IsFolder = true + }; + } + private static InternalItemsQuery CreateQuery() { // IncludeOwnedItems keeps the alternate version rows in the query so the @@ -78,13 +195,15 @@ public sealed class BaseItemRepositoryGroupingTests : SqliteDbTestFixture }; } - private BaseItemEntity CreateMovieEntity(Guid id, string name, string presentationKey, Guid? primaryVersionId) + private BaseItemEntity CreateMovieEntity(Guid id, string name, string presentationKey, Guid? primaryVersionId, Guid? libraryId = null) { return new BaseItemEntity { Id = id, Type = _movieTypeName, Name = name, + ParentId = libraryId, + TopParentId = libraryId, PresentationUniqueKey = presentationKey, PrimaryVersionId = primaryVersionId, MediaType = "Video", diff --git a/tests/Jellyfin.Server.Implementations.Tests/Item/ItemPersistenceAlternateVersionTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Item/ItemPersistenceAlternateVersionTests.cs new file mode 100644 index 0000000000..c15ea09965 --- /dev/null +++ b/tests/Jellyfin.Server.Implementations.Tests/Item/ItemPersistenceAlternateVersionTests.cs @@ -0,0 +1,191 @@ +using System; +using System.Globalization; +using System.Linq; +using System.Threading; +using Jellyfin.Server.Implementations.Item; +using MediaBrowser.Controller; +using MediaBrowser.Controller.Configuration; +using MediaBrowser.Controller.Entities; +using MediaBrowser.Controller.Entities.Movies; +using MediaBrowser.Controller.Library; +using MediaBrowser.Controller.LiveTv; +using MediaBrowser.Model.Configuration; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; +using DbLinkedChildType = Jellyfin.Database.Implementations.Entities.LinkedChildType; + +namespace Jellyfin.Server.Implementations.Tests.Item; + +/// <summary> +/// Covers the invariant that a video linked as an alternate version also carries the +/// PrimaryVersionId the item queries hide it by, including when it was already a library +/// item in its own right before it became a version. +/// </summary> +public sealed class ItemPersistenceAlternateVersionTests : SqliteDbTestFixture +{ + private const string PrimaryPath = "/movies/Movie/Movie - 4K.mkv"; + private const string VersionPath = "/movies/Movie/Movie - 1080p.mkv"; + + private readonly ItemPersistenceService _service; + private readonly ILibraryManager? _previousLibraryManager; + private readonly IServerConfigurationManager? _previousConfigurationManager; + private readonly IRecordingsManager? _previousRecordingsManager; + + public ItemPersistenceAlternateVersionTests() + { + // BaseItem resolves these through process-wide statics; restored in Dispose. + _previousLibraryManager = BaseItem.LibraryManager; + _previousConfigurationManager = BaseItem.ConfigurationManager; + _previousRecordingsManager = Video.RecordingsManager; + + var libraryManager = new Mock<ILibraryManager>(); + libraryManager.Setup(l => l.GetCollectionFolders(It.IsAny<BaseItem>())) + .Returns([]); + BaseItem.LibraryManager = libraryManager.Object; + + var configurationManager = new Mock<IServerConfigurationManager>(); + configurationManager.Setup(c => c.Configuration).Returns(new ServerConfiguration()); + BaseItem.ConfigurationManager = configurationManager.Object; + + // Video.SourceType asks this whether the file is an in-progress recording. + Video.RecordingsManager = new Mock<IRecordingsManager>().Object; + + // Paths round-trip through the host's virtual path mapping on the way in and out. + var appHost = new Mock<IServerApplicationHost>(); + appHost.Setup(h => h.ReverseVirtualPath(It.IsAny<string>())).Returns((string p) => p); + appHost.Setup(h => h.ExpandVirtualPath(It.IsAny<string>())).Returns((string p) => p); + + _service = new ItemPersistenceService( + CreateDbContextFactory(), + appHost.Object, + NullLogger<ItemPersistenceService>.Instance); + } + + protected override void Dispose(bool disposing) + { + BaseItem.LibraryManager = _previousLibraryManager!; + BaseItem.ConfigurationManager = _previousConfigurationManager!; + Video.RecordingsManager = _previousRecordingsManager!; + base.Dispose(disposing); + } + + [Fact] + public void SaveItems_LocalAlternateVersionAlreadyAnItem_SetsPrimaryVersionId() + { + var primaryId = Guid.Parse("aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"); + var versionId = Guid.Parse("bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb"); + + // The version was scanned as a standalone movie before it became a version, so it has a + // presentation key of its own and no PrimaryVersionId. + var version = CreateMovie(versionId, VersionPath); + version.PresentationUniqueKey = "standalone"; + _service.SaveItems([version], CancellationToken.None); + + using (var ctx = CreateDbContext()) + { + Assert.Null(ctx.BaseItems.First(e => e.Id.Equals(versionId)).PrimaryVersionId); + } + + // Now the scan folds it into a primary, which is the item that gets saved. + var primary = CreateMovie(primaryId, PrimaryPath); + primary.LocalAlternateVersions = [VersionPath]; + _service.SaveItems([primary], CancellationToken.None); + + using (var ctx = CreateDbContext()) + { + var link = Assert.Single(ctx.LinkedChildren.Where(e => e.ParentId.Equals(primaryId))); + Assert.Equal(DbLinkedChildType.LocalAlternateVersion, link.ChildType); + Assert.Equal(versionId, link.ChildId); + + var stored = ctx.BaseItems.First(e => e.Id.Equals(versionId)); + Assert.Equal(primaryId, stored.PrimaryVersionId); + + // Presentation-key grouping has to collapse it onto the primary as well. + Assert.Equal(primaryId.ToString("N", CultureInfo.InvariantCulture), stored.PresentationUniqueKey); + } + } + + [Fact] + public void SaveItems_LinkedAlternateVersionAlreadyAnItem_SetsPrimaryVersionId() + { + var primaryId = Guid.Parse("cccccccc-cccc-cccc-cccc-cccccccccccc"); + var versionId = Guid.Parse("dddddddd-dddd-dddd-dddd-dddddddddddd"); + + _service.SaveItems([CreateMovie(versionId, VersionPath)], CancellationToken.None); + + var primary = CreateMovie(primaryId, PrimaryPath); + primary.LinkedAlternateVersions = + [ + new LinkedChild { ItemId = versionId, Type = LinkedChildType.LinkedAlternateVersion } + ]; + _service.SaveItems([primary], CancellationToken.None); + + using var ctx = CreateDbContext(); + var link = Assert.Single(ctx.LinkedChildren.Where(e => e.ParentId.Equals(primaryId))); + Assert.Equal(DbLinkedChildType.LinkedAlternateVersion, link.ChildType); + Assert.Equal(primaryId, ctx.BaseItems.First(e => e.Id.Equals(versionId)).PrimaryVersionId); + } + + [Fact] + public void SaveItems_VersionAlreadyPointingAtPrimary_LeavesItAlone() + { + var primaryId = Guid.Parse("eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee"); + var versionId = Guid.Parse("ffffffff-ffff-ffff-ffff-ffffffffffff"); + + var version = CreateMovie(versionId, VersionPath); + version.SetPrimaryVersionId(primaryId); + _service.SaveItems([version], CancellationToken.None); + + var primary = CreateMovie(primaryId, PrimaryPath); + primary.LocalAlternateVersions = [VersionPath]; + _service.SaveItems([primary], CancellationToken.None); + + using var ctx = CreateDbContext(); + var stored = ctx.BaseItems.First(e => e.Id.Equals(versionId)); + Assert.Equal(primaryId, stored.PrimaryVersionId); + Assert.Equal(primaryId.ToString("N", CultureInfo.InvariantCulture), stored.PresentationUniqueKey); + } + + [Fact] + public void SaveItems_VideoListedAmongItsOwnVersions_KeepsItsOwnPrimaryVersionId() + { + var primaryId = Guid.Parse("11111111-1111-1111-1111-111111111111"); + + var primary = CreateMovie(primaryId, PrimaryPath); + primary.LocalAlternateVersions = [PrimaryPath]; + _service.SaveItems([primary], CancellationToken.None); + + using var ctx = CreateDbContext(); + Assert.Null(ctx.BaseItems.First(e => e.Id.Equals(primaryId)).PrimaryVersionId); + } + + [Fact] + public void SaveItems_PromotedVersionStillPointingAtOldPrimary_DoesNotCreateACycle() + { + var promotedId = Guid.Parse("22222222-2222-2222-2222-222222222222"); + var oldPrimaryId = Guid.Parse("33333333-3333-3333-3333-333333333333"); + + _service.SaveItems([CreateMovie(oldPrimaryId, VersionPath)], CancellationToken.None); + + // The rescan resolves this one as the primary of the group, but it still carries the pointer + // to the version it was promoted over. + var promoted = CreateMovie(promotedId, PrimaryPath); + promoted.SetPrimaryVersionId(oldPrimaryId); + promoted.LocalAlternateVersions = [VersionPath]; + _service.SaveItems([promoted], CancellationToken.None); + + using var ctx = CreateDbContext(); + + // Pointing the old primary back would hide both, and with them the whole group. + Assert.Null(ctx.BaseItems.First(e => e.Id.Equals(oldPrimaryId)).PrimaryVersionId); + Assert.Equal(oldPrimaryId, ctx.BaseItems.First(e => e.Id.Equals(promotedId)).PrimaryVersionId); + } + + private static Movie CreateMovie(Guid id, string path) => new() + { + Id = id, + Name = "Movie", + Path = path + }; +} diff --git a/tests/Jellyfin.Server.Implementations.Tests/Library/MovieSimilarItemsProviderTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Library/MovieSimilarItemsProviderTests.cs index 297f930205..421671b520 100644 --- a/tests/Jellyfin.Server.Implementations.Tests/Library/MovieSimilarItemsProviderTests.cs +++ b/tests/Jellyfin.Server.Implementations.Tests/Library/MovieSimilarItemsProviderTests.cs @@ -10,6 +10,7 @@ using Jellyfin.Database.Implementations.Entities; using Jellyfin.Server.Implementations.Tests.Item; using MediaBrowser.Controller.Configuration; using MediaBrowser.Controller.Dto; +using MediaBrowser.Controller.Entities; using MediaBrowser.Controller.Entities.Movies; using MediaBrowser.Controller.Library; using MediaBrowser.Model.Configuration; @@ -26,9 +27,14 @@ namespace Jellyfin.Server.Implementations.Tests.Library; /// </summary> public sealed class MovieSimilarItemsProviderTests : SqliteDbTestFixture { + private static readonly Guid _movieLibraryId = Guid.Parse("aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"); + private static readonly Guid _movie4KLibraryId = Guid.Parse("bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb"); + private readonly MovieSimilarItemsProvider _provider; + private readonly Mock<ILibraryManager> _libraryManager = new(); private readonly User _user = new("test", "auth-provider", "reset-provider"); private readonly string _movieTypeName; + private readonly string _folderTypeName; private readonly Guid _source = Guid.NewGuid(); private readonly Guid _sourceAlternate = Guid.NewGuid(); @@ -36,10 +42,19 @@ public sealed class MovieSimilarItemsProviderTests : SqliteDbTestFixture private readonly Guid _similarAlternate = Guid.NewGuid(); private readonly Guid _unrelated = Guid.NewGuid(); + // A second scenario, in two libraries and on a genre of its own, for the group whose primary the + // user may not be able to reach at all. + private readonly Guid _crossSource = Guid.NewGuid(); + private readonly Guid _crossLibraryPrimary = Guid.NewGuid(); + private readonly Guid _crossLibraryVersion = Guid.NewGuid(); + private readonly Guid _sameLibraryPrimary = Guid.NewGuid(); + private readonly Guid _sameLibraryVersion = Guid.NewGuid(); + public MovieSimilarItemsProviderTests() { var itemTypeLookup = new ItemTypeLookup(); _movieTypeName = itemTypeLookup.BaseItemKindNames[BaseItemKind.Movie]!; + _folderTypeName = itemTypeLookup.BaseItemKindNames[BaseItemKind.Folder]!; using (var context = CreateDbContext()) { @@ -53,7 +68,7 @@ public sealed class MovieSimilarItemsProviderTests : SqliteDbTestFixture CreateDbContextFactory(), CreateBaseItemRepository(itemTypeLookup), serverConfigurationManager.Object, - new Mock<ILibraryManager>().Object); + _libraryManager.Object); } [Fact] @@ -80,10 +95,52 @@ public sealed class MovieSimilarItemsProviderTests : SqliteDbTestFixture Assert.DoesNotContain(_sourceAlternate, items); } - private async Task<List<Guid>> GetSimilarItemsAsync() + [Fact] + public async Task GetSimilarItems_UserWithoutThePrimarysLibrary_OffersTheVersion() + { + // The user may only open the library the 1080p version is in, so its primary is out of reach + // and the version is all that is left to stand in for the group. + RestrictUserTo(_movieLibraryId); + + var items = await GetSimilarItemsAsync(_crossSource).ConfigureAwait(true); + + Assert.Contains(_crossLibraryVersion, items); + Assert.DoesNotContain(_crossLibraryPrimary, items); + } + + [Fact] + public async Task GetSimilarItems_UserWithBothLibraries_OffersThePrimaryOfTheGroupOnce() + { + RestrictUserTo(_movieLibraryId, _movie4KLibraryId); + + var items = await GetSimilarItemsAsync(_crossSource).ConfigureAwait(true); + + Assert.Contains(_crossLibraryPrimary, items); + Assert.DoesNotContain(_crossLibraryVersion, items); + } + + [Fact] + public async Task GetSimilarItems_GroupMergedInsideOneLibrary_StillOffersOnlyThePrimary() + { + RestrictUserTo(_movieLibraryId, _movie4KLibraryId); + + var items = await GetSimilarItemsAsync(_crossSource).ConfigureAwait(true); + + Assert.Contains(_sameLibraryPrimary, items); + Assert.DoesNotContain(_sameLibraryVersion, items); + } + + private void RestrictUserTo(params Guid[] libraryIds) + { + _libraryManager + .Setup(l => l.ConfigureUserAccess(It.IsAny<InternalItemsQuery>(), It.IsAny<User>())) + .Callback<InternalItemsQuery, User>((query, _) => query.TopParentIds = libraryIds); + } + + private async Task<List<Guid>> GetSimilarItemsAsync(Guid? sourceId = null) { var results = await _provider.GetSimilarItemsAsync( - new Movie { Id = _source, Name = "Source" }, + new Movie { Id = sourceId ?? _source, Name = "Source" }, new SimilarItemsQuery { User = _user, Limit = 10, DtoOptions = new DtoOptions() }, CancellationToken.None).ConfigureAwait(false); @@ -102,19 +159,52 @@ public sealed class MovieSimilarItemsProviderTests : SqliteDbTestFixture var similarAlternate = AddMovie(context, _similarAlternate, "Similar 4K", primaryVersionId: _similar); var unrelated = AddMovie(context, _unrelated, "Unrelated", primaryVersionId: null); + // The second scenario scores on a genre of its own, so it stays out of the results above. + var crossLibrary = CreateItemValue("Science Fiction", "science fiction"); + + AddLibrary(context, _movieLibraryId, "Movies"); + AddLibrary(context, _movie4KLibraryId, "Movies-4K"); + + var crossSource = AddMovie(context, _crossSource, "Cross Source", primaryVersionId: null, libraryId: _movieLibraryId); + + // The 4K version heads the group and lives in a library of its own. + var crossLibraryPrimary = AddMovie(context, _crossLibraryPrimary, "Coco 4K", primaryVersionId: null, libraryId: _movie4KLibraryId); + var crossLibraryVersion = AddMovie(context, _crossLibraryVersion, "Coco", primaryVersionId: _crossLibraryPrimary, libraryId: _movieLibraryId); + + // A group merged inside one library, as a control. + var sameLibraryPrimary = AddMovie(context, _sameLibraryPrimary, "Up 4K", primaryVersionId: null, libraryId: _movieLibraryId); + var sameLibraryVersion = AddMovie(context, _sameLibraryVersion, "Up", primaryVersionId: _sameLibraryPrimary, libraryId: _movieLibraryId); + context.Users.Add(_user); - context.ItemValues.AddRange(shared, other); + context.ItemValues.AddRange(shared, other, crossLibrary); context.ItemValuesMap.AddRange( CreateMap(source, shared), CreateMap(sourceAlternate, shared), CreateMap(similar, shared), CreateMap(similarAlternate, shared), - CreateMap(unrelated, other)); + CreateMap(unrelated, other), + CreateMap(crossSource, crossLibrary), + CreateMap(crossLibraryPrimary, crossLibrary), + CreateMap(crossLibraryVersion, crossLibrary), + CreateMap(sameLibraryPrimary, crossLibrary), + CreateMap(sameLibraryVersion, crossLibrary)); context.SaveChanges(); } - private BaseItemEntity AddMovie(JellyfinDbContext context, Guid id, string name, Guid? primaryVersionId) + private void AddLibrary(JellyfinDbContext context, Guid id, string name) + { + context.BaseItems.Add(new BaseItemEntity + { + Id = id, + Type = _folderTypeName, + Name = name, + Path = "/" + name, + IsFolder = true + }); + } + + private BaseItemEntity AddMovie(JellyfinDbContext context, Guid id, string name, Guid? primaryVersionId, Guid? libraryId = null) { var item = new BaseItemEntity { @@ -122,6 +212,8 @@ public sealed class MovieSimilarItemsProviderTests : SqliteDbTestFixture Type = _movieTypeName, Name = name, SortName = name, + ParentId = libraryId, + TopParentId = libraryId, MediaType = "Video", IsMovie = true, IsFolder = false, diff --git a/tests/Jellyfin.Server.Implementations.Tests/Library/SqlSearchProviderTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Library/SqlSearchProviderTests.cs new file mode 100644 index 0000000000..5aa770b9b3 --- /dev/null +++ b/tests/Jellyfin.Server.Implementations.Tests/Library/SqlSearchProviderTests.cs @@ -0,0 +1,126 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Emby.Server.Implementations.Data; +using Emby.Server.Implementations.Library.Search; +using Jellyfin.Database.Implementations; +using Jellyfin.Database.Implementations.Entities; +using Jellyfin.Server.Implementations.Item; +using Jellyfin.Server.Implementations.Tests.Item; +using MediaBrowser.Controller.Entities; +using MediaBrowser.Controller.Library; +using Moq; +using Xunit; +using BaseItemKind = Jellyfin.Data.Enums.BaseItemKind; + +namespace Jellyfin.Server.Implementations.Tests.Library; + +/// <summary> +/// Covers what <see cref="SqlSearchProvider"/> returns for a version group merged across two +/// libraries: the primary represents the group wherever it is visible, and the version stands in +/// for it for a user who cannot open the library the primary lives in. +/// </summary> +public sealed class SqlSearchProviderTests : SqliteDbTestFixture +{ + private static readonly Guid _movieLibraryId = Guid.Parse("aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"); + private static readonly Guid _movie4KLibraryId = Guid.Parse("bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb"); + private static readonly Guid _primaryId = Guid.Parse("cccccccc-cccc-cccc-cccc-cccccccccccc"); + private static readonly Guid _versionId = Guid.Parse("dddddddd-dddd-dddd-dddd-dddddddddddd"); + + private readonly SqlSearchProvider _provider; + private readonly Mock<ILibraryManager> _libraryManager = new(); + private readonly User _user = new("test", "auth-provider", "reset-provider"); + + public SqlSearchProviderTests() + { + var itemTypeLookup = new ItemTypeLookup(); + var movieTypeName = itemTypeLookup.BaseItemKindNames[BaseItemKind.Movie]!; + var folderTypeName = itemTypeLookup.BaseItemKindNames[BaseItemKind.Folder]!; + + using (var context = CreateDbContext()) + { + context.Users.Add(_user); + context.BaseItems.Add(CreateLibrary(_movieLibraryId, folderTypeName, "Movies", "/movies")); + context.BaseItems.Add(CreateLibrary(_movie4KLibraryId, folderTypeName, "Movies-4K", "/movies-4k")); + context.BaseItems.Add(CreateMovie(_primaryId, movieTypeName, _movie4KLibraryId, null)); + context.BaseItems.Add(CreateMovie(_versionId, movieTypeName, _movieLibraryId, _primaryId)); + context.SaveChanges(); + } + + var userManager = new Mock<IUserManager>(); + userManager.Setup(u => u.GetUserById(_user.Id)).Returns(_user); + + _provider = new SqlSearchProvider( + CreateDbContextFactory(), + itemTypeLookup, + _libraryManager.Object, + userManager.Object, + CreateBaseItemRepository(itemTypeLookup)); + } + + [Fact] + public async Task SearchAsync_UserWithoutThePrimarysLibrary_FindsTheVersion() + { + RestrictUserTo(_movieLibraryId); + + var hits = await SearchAsync().ConfigureAwait(true); + + Assert.Equal([_versionId], hits); + } + + [Fact] + public async Task SearchAsync_UserWithBothLibraries_FindsThePrimaryOnce() + { + RestrictUserTo(_movieLibraryId, _movie4KLibraryId); + + var hits = await SearchAsync().ConfigureAwait(true); + + Assert.Equal([_primaryId], hits); + } + + private void RestrictUserTo(params Guid[] libraryIds) + { + _libraryManager + .Setup(l => l.ConfigureUserAccess(It.IsAny<InternalItemsQuery>(), It.IsAny<User>())) + .Callback<InternalItemsQuery, User>((query, _) => query.TopParentIds = libraryIds); + } + + private async Task<List<Guid>> SearchAsync() + { + var results = await _provider.SearchAsync( + new SearchProviderQuery { SearchTerm = "coco", UserId = _user.Id, Limit = 10 }, + CancellationToken.None).ConfigureAwait(false); + + return results.Select(r => r.ItemId).ToList(); + } + + private static BaseItemEntity CreateLibrary(Guid id, string typeName, string name, string path) + => new() + { + Id = id, + Type = typeName, + Name = name, + Path = path, + IsFolder = true + }; + + private static BaseItemEntity CreateMovie(Guid id, string typeName, Guid libraryId, Guid? primaryVersionId) + => new() + { + Id = id, + Type = typeName, + Name = "Coco", + CleanName = "coco", + SortName = "Coco", + MediaType = "Video", + IsMovie = true, + IsFolder = false, + IsVirtualItem = false, + ParentId = libraryId, + TopParentId = libraryId, + PresentationUniqueKey = (primaryVersionId ?? id).ToString("N"), + PrimaryVersionId = primaryVersionId + }; +} diff --git a/tests/Jellyfin.Server.Tests/Migrations/RepairAlternateVersionLinksTests.cs b/tests/Jellyfin.Server.Tests/Migrations/RepairAlternateVersionLinksTests.cs new file mode 100644 index 0000000000..34eff8a988 --- /dev/null +++ b/tests/Jellyfin.Server.Tests/Migrations/RepairAlternateVersionLinksTests.cs @@ -0,0 +1,216 @@ +using System; +using System.Globalization; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Jellyfin.Database.Implementations; +using Jellyfin.Database.Implementations.Entities; +using Jellyfin.Database.Implementations.Locking; +using Jellyfin.Database.Providers.Sqlite; +using Jellyfin.Server.Migrations.Routines; +using Jellyfin.Server.ServerSetupApp; +using MediaBrowser.Common.Configuration; +using Microsoft.Data.Sqlite; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; + +namespace Jellyfin.Server.Tests.Migrations; + +/// <summary> +/// Covers the repair of the PrimaryVersionId the item queries hide an alternate version by, +/// including the link shapes that would otherwise leave a whole version group hidden. +/// </summary> +public sealed class RepairAlternateVersionLinksTests : IDisposable +{ + private readonly SqliteConnection _connection; + private readonly DbContextOptions<JellyfinDbContext> _dbOptions; + private readonly IApplicationPaths _applicationPaths; + + public RepairAlternateVersionLinksTests() + { + _applicationPaths = new Mock<IApplicationPaths>().Object; + + // The connection owns the in-memory database, so it stays open for the whole test. + _connection = new SqliteConnection("Data Source=:memory:"); + _connection.Open(); + + _dbOptions = new DbContextOptionsBuilder<JellyfinDbContext>() + .UseSqlite(_connection) + .Options; + + using var context = CreateDbContext(); + context.Database.EnsureCreated(); + } + + [Fact] + public async Task PerformAsync_VersionLinkedToPrimary_PointsItAtThePrimary() + { + var primaryId = Guid.Parse("aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"); + var versionId = Guid.Parse("bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb"); + + Seed( + [(primaryId, null), (versionId, null)], + [(primaryId, versionId)]); + + await PerformAsync(); + + using var context = CreateDbContext(); + AssertIsVersionOf(context, versionId, primaryId); + AssertIsPrimary(context, primaryId); + } + + [Fact] + public async Task PerformAsync_VideosLinkedAsEachOthersVersion_KeepsOneOfThemVisible() + { + var firstId = Guid.Parse("cccccccc-cccc-cccc-cccc-cccccccccccc"); + var secondId = Guid.Parse("dddddddd-dddd-dddd-dddd-dddddddddddd"); + + // Each one claims the other as its version, so pointing both at their link would hide the + // group in its entirety. + Seed( + [(firstId, null), (secondId, null)], + [(firstId, secondId), (secondId, firstId)]); + + await PerformAsync(); + + using var context = CreateDbContext(); + var first = Get(context, firstId); + var second = Get(context, secondId); + + var primary = first.PrimaryVersionId is null ? first : second; + var version = first.PrimaryVersionId is null ? second : first; + + Assert.Null(primary.PrimaryVersionId); + AssertIsPrimary(context, primary.Id); + AssertIsVersionOf(context, version.Id, primary.Id); + } + + [Fact] + public async Task PerformAsync_PrimaryStillPointingAtItsOwnVersion_ClearsTheStalePrimary() + { + var primaryId = Guid.Parse("eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee"); + var versionId = Guid.Parse("ffffffff-ffff-ffff-ffff-ffffffffffff"); + + // The primary was promoted over the version it now heads, but kept the pointer to it: the + // repair below would point the version back and leave both hidden. + Seed( + [(primaryId, versionId), (versionId, null)], + [(primaryId, versionId)]); + + await PerformAsync(); + + using var context = CreateDbContext(); + AssertIsPrimary(context, primaryId); + AssertIsVersionOf(context, versionId, primaryId); + } + + [Fact] + public async Task PerformAsync_ChainedLinks_PointsEveryVersionAtTheHeadOfTheChain() + { + var headId = Guid.Parse("11111111-1111-1111-1111-111111111111"); + var middleId = Guid.Parse("22222222-2222-2222-2222-222222222222"); + var tailId = Guid.Parse("33333333-3333-3333-3333-333333333333"); + + // The middle one is a version of the head and a primary of the tail at the same time. + Seed( + [(headId, null), (middleId, null), (tailId, null)], + [(headId, middleId), (middleId, tailId)]); + + await PerformAsync(); + + using var context = CreateDbContext(); + AssertIsPrimary(context, headId); + AssertIsVersionOf(context, middleId, headId); + AssertIsVersionOf(context, tailId, headId); + } + + [Fact] + public async Task PerformAsync_VersionLinkedToItself_LeavesItVisible() + { + var itemId = Guid.Parse("44444444-4444-4444-4444-444444444444"); + + Seed([(itemId, null)], [(itemId, itemId)]); + + await PerformAsync(); + + using var context = CreateDbContext(); + Assert.Null(Get(context, itemId).PrimaryVersionId); + } + + public void Dispose() + { + _connection.Dispose(); + } + + private static void AssertIsPrimary(JellyfinDbContext context, Guid id) + { + var item = Get(context, id); + Assert.Null(item.PrimaryVersionId); + Assert.Equal(id.ToString("N", CultureInfo.InvariantCulture), item.PresentationUniqueKey); + } + + private static void AssertIsVersionOf(JellyfinDbContext context, Guid id, Guid primaryId) + { + var item = Get(context, id); + Assert.Equal(primaryId, item.PrimaryVersionId); + + // Presentation-key grouping has to collapse the version onto its primary as well. + Assert.Equal(primaryId.ToString("N", CultureInfo.InvariantCulture), item.PresentationUniqueKey); + } + + private static BaseItemEntity Get(JellyfinDbContext context, Guid id) + => context.BaseItems.AsNoTracking().First(e => e.Id.Equals(id)); + + private JellyfinDbContext CreateDbContext() => new( + _dbOptions, + NullLogger<JellyfinDbContext>.Instance, + new SqliteDatabaseProvider(_applicationPaths, NullLogger<SqliteDatabaseProvider>.Instance), + new NoLockBehavior(NullLogger<NoLockBehavior>.Instance)); + + private void Seed( + (Guid Id, Guid? PrimaryVersionId)[] items, + (Guid ParentId, Guid ChildId)[] links) + { + using var context = CreateDbContext(); + + foreach (var (id, primaryVersionId) in items) + { + context.BaseItems.Add(new BaseItemEntity + { + Id = id, + Type = "MediaBrowser.Controller.Entities.Movies.Movie", + Name = "Movie", + PrimaryVersionId = primaryVersionId, + PresentationUniqueKey = (primaryVersionId ?? id).ToString("N", CultureInfo.InvariantCulture) + }); + } + + foreach (var (parentId, childId) in links) + { + context.LinkedChildren.Add(new LinkedChildEntity + { + ParentId = parentId, + ChildId = childId, + ChildType = LinkedChildType.LinkedAlternateVersion + }); + } + + context.SaveChanges(); + } + + private Task PerformAsync() + { + var factory = new Mock<IDbContextFactory<JellyfinDbContext>>(); + factory.Setup(f => f.CreateDbContext()).Returns(CreateDbContext); + factory.Setup(f => f.CreateDbContextAsync(It.IsAny<CancellationToken>())) + .ReturnsAsync(CreateDbContext); + + var migration = new RepairAlternateVersionLinks( + new StartupLogger<RepairAlternateVersionLinks>(NullLogger<RepairAlternateVersionLinks>.Instance), + factory.Object); + + return migration.PerformAsync(CancellationToken.None); + } +} |