aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorbrandon <brandon@clinger.dev>2026-08-08 12:33:11 -0400
committerbrandon <brandon@clinger.dev>2026-08-08 12:33:11 -0400
commit10d108a1f453c0972b3059e10bb9b13bbcb34f63 (patch)
treeb00f02ff96a21a78b35ca69ef5614466e0afc3ca
parentc091ffdc6b2d8d4dd6f439d056c561bae7bd9a18 (diff)
Address review on MediaSourceCount batching
Rename GetItemsWithAlternateVersions to GetItemIdsWithAlternateVersions across the interfaces and implementations since it returns ids. Return the hashset straight from the query instead of materializing an array first. Rename the DtoService guard to mayHaveAlternateVersions and invert it so the computed path is the explicit case. Assert the media source count value in the batch skip test and add a test covering an item that is in the returned set still resolving to the correct count.
-rw-r--r--Emby.Server.Implementations/Dto/DtoService.cs18
-rw-r--r--Emby.Server.Implementations/Library/LibraryManager.cs4
-rw-r--r--Jellyfin.Server.Implementations/Item/LinkedChildrenService.cs8
-rw-r--r--MediaBrowser.Controller/Library/ILibraryManager.cs2
-rw-r--r--MediaBrowser.Controller/Persistence/ILinkedChildrenService.cs2
-rw-r--r--tests/Jellyfin.Server.Implementations.Tests/Dto/DtoServiceImageInheritanceTests.cs47
6 files changed, 61 insertions, 20 deletions
diff --git a/Emby.Server.Implementations/Dto/DtoService.cs b/Emby.Server.Implementations/Dto/DtoService.cs
index 062c19a1d4..6fa057702c 100644
--- a/Emby.Server.Implementations/Dto/DtoService.cs
+++ b/Emby.Server.Implementations/Dto/DtoService.cs
@@ -261,7 +261,7 @@ namespace Emby.Server.Implementations.Dto
var versionItemIds = accessibleItems.OfType<Video>().Select(i => i.Id).ToList();
if (versionItemIds.Count > 0)
{
- alternateVersionItemIds = _libraryManager.GetItemsWithAlternateVersions(versionItemIds);
+ alternateVersionItemIds = _libraryManager.GetItemIdsWithAlternateVersions(versionItemIds);
}
}
@@ -1314,13 +1314,15 @@ namespace Emby.Server.Implementations.Dto
if (options.ContainsField(ItemFields.MediaSourceCount))
{
// A video with no primary version and no alternate versions always has a single
- // media source. When the batch has already determined this item owns no alternate
- // versions, skip the per-item alternate-version queries entirely (the common case).
- var hasNoAlternateVersions = alternateVersionItemIds is not null
- && !video.PrimaryVersionId.HasValue
- && !alternateVersionItemIds.Contains(video.Id);
-
- if (!hasNoAlternateVersions)
+ // media source. Only compute the count for videos that might have more: a primary
+ // version, or membership in the batch's set of items that own alternate versions.
+ // Without the batch we can't rule it out, so fall back to computing (the single-item
+ // path). Everything else is the common case and keeps the default count of one.
+ var mayHaveAlternateVersions = alternateVersionItemIds is null
+ || video.PrimaryVersionId.HasValue
+ || alternateVersionItemIds.Contains(video.Id);
+
+ if (mayHaveAlternateVersions)
{
// Match the per-user filtering of the media sources: versions the user cannot
// access are not selectable, so they must not count towards the badge either.
diff --git a/Emby.Server.Implementations/Library/LibraryManager.cs b/Emby.Server.Implementations/Library/LibraryManager.cs
index 9bb962c504..0c7c411d0c 100644
--- a/Emby.Server.Implementations/Library/LibraryManager.cs
+++ b/Emby.Server.Implementations/Library/LibraryManager.cs
@@ -2235,9 +2235,9 @@ namespace Emby.Server.Implementations.Library
}
/// <inheritdoc />
- public IReadOnlySet<Guid> GetItemsWithAlternateVersions(IReadOnlyList<Guid> itemIds)
+ public IReadOnlySet<Guid> GetItemIdsWithAlternateVersions(IReadOnlyList<Guid> itemIds)
{
- return _linkedChildrenService.GetItemsWithAlternateVersions(itemIds);
+ return _linkedChildrenService.GetItemIdsWithAlternateVersions(itemIds);
}
/// <inheritdoc />
diff --git a/Jellyfin.Server.Implementations/Item/LinkedChildrenService.cs b/Jellyfin.Server.Implementations/Item/LinkedChildrenService.cs
index 2452f8e3c6..a96e941c88 100644
--- a/Jellyfin.Server.Implementations/Item/LinkedChildrenService.cs
+++ b/Jellyfin.Server.Implementations/Item/LinkedChildrenService.cs
@@ -60,7 +60,7 @@ public class LinkedChildrenService : ILinkedChildrenService
}
/// <inheritdoc/>
- public IReadOnlySet<Guid> GetItemsWithAlternateVersions(IReadOnlyList<Guid> itemIds)
+ public IReadOnlySet<Guid> GetItemIdsWithAlternateVersions(IReadOnlyList<Guid> itemIds)
{
if (itemIds.Count == 0)
{
@@ -69,15 +69,13 @@ public class LinkedChildrenService : ILinkedChildrenService
using var dbContext = _dbProvider.CreateDbContext();
- var parentIds = dbContext.LinkedChildren
+ return dbContext.LinkedChildren
.Where(lc => (lc.ChildType == DbLinkedChildType.LocalAlternateVersion
|| lc.ChildType == DbLinkedChildType.LinkedAlternateVersion)
&& itemIds.Contains(lc.ParentId))
.Select(lc => lc.ParentId)
.Distinct()
- .ToArray();
-
- return parentIds.ToHashSet();
+ .ToHashSet();
}
/// <inheritdoc/>
diff --git a/MediaBrowser.Controller/Library/ILibraryManager.cs b/MediaBrowser.Controller/Library/ILibraryManager.cs
index f6bd948b99..ca686fbd9d 100644
--- a/MediaBrowser.Controller/Library/ILibraryManager.cs
+++ b/MediaBrowser.Controller/Library/ILibraryManager.cs
@@ -261,7 +261,7 @@ namespace MediaBrowser.Controller.Library
/// </summary>
/// <param name="itemIds">The item IDs to check.</param>
/// <returns>The set of item IDs that have alternate versions.</returns>
- IReadOnlySet<Guid> GetItemsWithAlternateVersions(IReadOnlyList<Guid> itemIds);
+ IReadOnlySet<Guid> GetItemIdsWithAlternateVersions(IReadOnlyList<Guid> itemIds);
/// <summary>
/// Creates or updates a LinkedChild entry linking a parent to a child item.
diff --git a/MediaBrowser.Controller/Persistence/ILinkedChildrenService.cs b/MediaBrowser.Controller/Persistence/ILinkedChildrenService.cs
index c1fe3231f4..79c29410e4 100644
--- a/MediaBrowser.Controller/Persistence/ILinkedChildrenService.cs
+++ b/MediaBrowser.Controller/Persistence/ILinkedChildrenService.cs
@@ -26,7 +26,7 @@ public interface ILinkedChildrenService
/// </summary>
/// <param name="itemIds">The item IDs to check.</param>
/// <returns>The set of item IDs that have alternate versions.</returns>
- IReadOnlySet<Guid> GetItemsWithAlternateVersions(IReadOnlyList<Guid> itemIds);
+ IReadOnlySet<Guid> GetItemIdsWithAlternateVersions(IReadOnlyList<Guid> itemIds);
/// <summary>
/// Gets all artist matches from the database.
diff --git a/tests/Jellyfin.Server.Implementations.Tests/Dto/DtoServiceImageInheritanceTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Dto/DtoServiceImageInheritanceTests.cs
index fa94250287..6a3dcab57a 100644
--- a/tests/Jellyfin.Server.Implementations.Tests/Dto/DtoServiceImageInheritanceTests.cs
+++ b/tests/Jellyfin.Server.Implementations.Tests/Dto/DtoServiceImageInheritanceTests.cs
@@ -222,11 +222,11 @@ public class DtoServiceImageInheritanceTests
var libraryManager = new Mock<ILibraryManager>();
// DtoService detects which videos own alternate versions in ONE batch
- // (GetItemsWithAlternateVersions) before the per-item loop. Videos absent from that set have a
+ // (GetItemIdsWithAlternateVersions) before the per-item loop. Videos absent from that set have a
// single media source, so the per-item GetLinkedAlternateVersions/GetLocalAlternateVersionIds
// queries (the N+1) must be skipped entirely. Here neither movie has alternate versions.
libraryManager
- .Setup(x => x.GetItemsWithAlternateVersions(It.IsAny<IReadOnlyList<Guid>>()))
+ .Setup(x => x.GetItemIdsWithAlternateVersions(It.IsAny<IReadOnlyList<Guid>>()))
.Returns(new HashSet<Guid>());
var dtoService = BuildDtoService(libraryManager);
@@ -236,13 +236,54 @@ public class DtoServiceImageInheritanceTests
Assert.Equal(2, dtos.Count);
+ // A single media source is the default, so the count is left unset (the client treats null as one).
+ foreach (var dto in dtos)
+ {
+ Assert.Null(dto.MediaSourceCount);
+ }
+
// The alternate-version check is batched once for the whole set, and the per-item lookups are
// never reached because the batch already ruled out alternate versions.
- libraryManager.Verify(x => x.GetItemsWithAlternateVersions(It.IsAny<IReadOnlyList<Guid>>()), Times.Once);
+ libraryManager.Verify(x => x.GetItemIdsWithAlternateVersions(It.IsAny<IReadOnlyList<Guid>>()), Times.Once);
libraryManager.Verify(x => x.GetLinkedAlternateVersions(It.IsAny<Video>()), Times.Never);
libraryManager.Verify(x => x.GetLocalAlternateVersionIds(It.IsAny<Video>()), Times.Never);
}
+ [Fact]
+ public void GetBaseItemDtos_VideoInAlternateVersionBatch_ResolvesRealCount()
+ {
+ var movie = new Movie
+ {
+ Id = Guid.NewGuid(),
+ Name = "Movie",
+ ImageInfos = []
+ };
+
+ var libraryManager = new Mock<ILibraryManager>();
+
+ // This movie IS in the batch set, so the fast path must not short-circuit it: the per-item
+ // lookups still run and the count is computed exactly as it was before batching. Two linked
+ // alternate versions plus the movie itself is a count of three.
+ libraryManager
+ .Setup(x => x.GetItemIdsWithAlternateVersions(It.IsAny<IReadOnlyList<Guid>>()))
+ .Returns(new HashSet<Guid> { movie.Id });
+ libraryManager
+ .Setup(x => x.GetLinkedAlternateVersions(It.IsAny<Video>()))
+ .Returns([new Movie { Id = Guid.NewGuid() }, new Movie { Id = Guid.NewGuid() }]);
+ libraryManager
+ .Setup(x => x.GetLocalAlternateVersionIds(It.IsAny<Video>()))
+ .Returns([]);
+
+ var dtoService = BuildDtoService(libraryManager);
+
+ var options = new DtoOptions(false) { Fields = [ItemFields.MediaSourceCount] };
+ var dtos = dtoService.GetBaseItemDtos([movie], options);
+
+ Assert.Single(dtos);
+ Assert.Equal(3, dtos[0].MediaSourceCount);
+ libraryManager.Verify(x => x.GetItemIdsWithAlternateVersions(It.IsAny<IReadOnlyList<Guid>>()), Times.Once);
+ }
+
private static DtoService BuildDtoService(BaseItem displayParent)
{
var libraryManager = new Mock<ILibraryManager>();