From 8150ee2484dabed61718ef43d5c98cdd2c0fa01c Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Tue, 15 Sep 2026 11:13:48 -0400 Subject: Backport pull request #17842 from jellyfin/release-12.z Fix versions of a video still listing separately from their group and preserve manual merges Original-merge: cabec7ec28df671e3bb1c360c32db60b085f4084 Merged-by: crobibero Backported-by: Cody Robibero --- .../Library/Search/SearchManager.cs | 14 +- .../Library/Search/SqlSearchProvider.cs | 7 + .../SimilarItems/MovieSimilarItemsProvider.cs | 16 +- .../Item/BaseItemRepository.QueryBuilding.cs | 13 +- .../Item/BaseItemRepository.TranslateQuery.cs | 15 +- .../Item/ItemPersistenceService.cs | 33 ++++ .../20260908120000_RepairAlternateVersionLinks.cs | 216 +++++++++++++++++++++ MediaBrowser.Controller/Entities/Folder.cs | 53 ++++- .../Entities/InternalItemsQuery.cs | 8 + .../Entities/BaseItemTests.cs | 18 ++ .../Item/BaseItemRepositoryChildrenTests.cs | 103 ++++++++++ .../Item/BaseItemRepositoryGroupingTests.cs | 121 +++++++++++- .../Item/ItemPersistenceAlternateVersionTests.cs | 191 ++++++++++++++++++ .../Library/MovieSimilarItemsProviderTests.cs | 104 +++++++++- .../Library/SqlSearchProviderTests.cs | 126 ++++++++++++ .../Migrations/RepairAlternateVersionLinksTests.cs | 216 +++++++++++++++++++++ 16 files changed, 1226 insertions(+), 28 deletions(-) create mode 100644 Jellyfin.Server/Migrations/Routines/20260908120000_RepairAlternateVersionLinks.cs create mode 100644 tests/Jellyfin.Server.Implementations.Tests/Item/BaseItemRepositoryChildrenTests.cs create mode 100644 tests/Jellyfin.Server.Implementations.Tests/Item/ItemPersistenceAlternateVersionTests.cs create mode 100644 tests/Jellyfin.Server.Implementations.Tests/Library/SqlSearchProviderTests.cs create mode 100644 tests/Jellyfin.Server.Tests/Migrations/RepairAlternateVersionLinksTests.cs 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 ExcludeVersionsOfMatchedPrimaries(IQueryable query) + { + var matched = query; + return query.Where(e => e.PrimaryVersionId == null || !matched.Any(p => p.Id == e.PrimaryVersionId)); + } + private IQueryable ApplyUserAccessFilter( JellyfinDbContext dbContext, IQueryable 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 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 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 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 !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 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 ApplyAlternateVersionFiltering( + JellyfinDbContext context, + IQueryable baseQuery) + => baseQuery.Where(e => e.PrimaryVersionId == null + || !context.BaseItems.Any(p => p.Id == e.PrimaryVersionId && p.TopParentId == e.TopParentId)); + /// /// Restricts a query to the libraries the user may open, exempting requested by-name items. /// 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; + +/// +/// Re-points every video that is linked as an alternate version at the primary it belongs to. +/// +[JellyfinMigration("2026-09-08T12:00:00", nameof(RepairAlternateVersionLinks))] +[JellyfinMigrationBackup(JellyfinDb = true)] +internal class RepairAlternateVersionLinks : IAsyncMigrationRoutine +{ + private const int BatchSize = 1000; + + private readonly IStartupLogger _logger; + private readonly IDbContextFactory _dbProvider; + + /// + /// Initializes a new instance of the class. + /// + /// The startup logger. + /// The database context factory. + public RepairAlternateVersionLinks( + IStartupLogger logger, + IDbContextFactory dbProvider) + { + _logger = logger; + _dbProvider = dbProvider; + } + + /// + 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 primaryByChild) + { + var resolvedPrimaries = new Dictionary(primaryByChild.Count); + + foreach (var start in primaryByChild.Keys.ToList()) + { + if (resolvedPrimaries.ContainsKey(start)) + { + continue; + } + + var chain = new List(); + var walked = new HashSet(); + 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(); 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