From 2cf90fb44fc915ec9933ab024d20d8e143717be5 Mon Sep 17 00:00:00 2001 From: Timothy Date: Mon, 20 Jul 2026 16:34:51 +0000 Subject: [PATCH] feat(489): support Jellyfin mixed-content libraries (#493) Jellyfin libraries typed `mixed` were dropped by JellyfinApiClient.Project's `_ => None` with no log line, so music and standup content could not be ingested without a local-library workaround that bypassed Jellyfin entirely. Adds LibraryMediaKind.Mixed, maps "mixed"/absent/blank CollectionType onto it, and gives SynchronizeJellyfinLibraryByIdHandler a Mixed arm composing the three existing per-kind scanners. Jellyfin classifies items server-side via includeItemTypes, so the passes see disjoint sets; reconciliation is type-scoped and cannot cross-delete. No new scanner and no DB migration -- MediaItem is TPT keyed on LibraryPathId, so heterogeneous contents were already legal. Segregation falls out of the model: a library is a place (one path <-> one Jellyfin library <-> one ErsatzTV library), so music/standup cannot leak into Movies or TV Shows. Also removes the silent-success `_ => Unit.Default` from both scanner dispatchers, which returned Right for an unhandled kind and stamped LastScan as though a scan had run, and rejects Mixed for local libraries at the API. Deliberately Jellyfin-only: local scanners share one video extension list and would claim each other's files, and LibraryFolder etags are keyed by LibraryPathId with no notion of kind. Verified by live E2E against a real Jellyfin, including the interaction with #494's reconciliation sweep. Four cold review rounds, all MERGEABLE. fixes #489 Co-authored-by: Timothy Co-committed-by: Timothy --- .../Commands/CreateLocalLibraryHandler.cs | 16 +- .../Domain/Library/LibraryMediaKind.cs | 12 +- .../Jellyfin/JellyfinApiClientTests.cs | 135 +++++++++ .../Jellyfin/JellyfinApiClient.cs | 21 +- ...chronizeJellyfinLibraryByIdHandlerTests.cs | 264 +++++++++++++++++- .../SynchronizeJellyfinLibraryByIdHandler.cs | 72 ++++- .../SynchronizeJellyfinShowByIdHandler.cs | 3 +- .../Commands/ScanLocalLibraryHandler.cs | 6 +- .../CreateLocalLibraryHandlerTests.cs | 22 ++ .../Api/LocalLibrariesController.cs | 4 + ErsatzTV/wwwroot/openapi/v1.json | 4 +- docs/decisions.md | 71 +++++ web/src/api/generated/v1.d.ts | 2 +- web/src/screens/LibrariesScreen.test.tsx | 29 +- 14 files changed, 643 insertions(+), 18 deletions(-) diff --git a/ErsatzTV.Application/Libraries/Commands/CreateLocalLibraryHandler.cs b/ErsatzTV.Application/Libraries/Commands/CreateLocalLibraryHandler.cs index 0f5976a34..e91d14c3e 100644 --- a/ErsatzTV.Application/Libraries/Commands/CreateLocalLibraryHandler.cs +++ b/ErsatzTV.Application/Libraries/Commands/CreateLocalLibraryHandler.cs @@ -1,4 +1,4 @@ -using System.IO.Abstractions; +using System.IO.Abstractions; using System.Threading.Channels; using ErsatzTV.Application.MediaSources; using ErsatzTV.Core; @@ -70,9 +70,23 @@ public class CreateLocalLibraryHandler : LocalLibraryHandlerBase, CreateLocalLibrary request) => MediaSourceMustExist(dbContext, request) .BindT(localLibrary => NameMustBeValid(request, localLibrary)) + .BindT(MediaKindMustBeSupportedLocally) .BindT(localLibrary => PathsMustBeValid(dbContext, localLibrary)) .BindT(localLibrary => NewPathsMustExist(fileSystem, localLibrary)); + /// + /// Mixed is only ever produced for remote (Jellyfin) libraries, where the media server classifies + /// each item for us. No local folder scanner handles it, so a local Mixed library would fail every + /// scan forever. The API takes a raw LibraryMediaKind, so this must be enforced here rather than + /// left to the SPA's media-kind options. + /// + private static Validation MediaKindMustBeSupportedLocally( + LocalLibrary localLibrary) => + localLibrary.MediaKind is LibraryMediaKind.Mixed + ? BaseError.New( + "Local libraries cannot use the Mixed media kind; it is only valid for Jellyfin libraries.") + : localLibrary; + private static Task> MediaSourceMustExist( TvContext dbContext, CreateLocalLibrary request) => diff --git a/ErsatzTV.Core/Domain/Library/LibraryMediaKind.cs b/ErsatzTV.Core/Domain/Library/LibraryMediaKind.cs index d31834026..934b46c88 100644 --- a/ErsatzTV.Core/Domain/Library/LibraryMediaKind.cs +++ b/ErsatzTV.Core/Domain/Library/LibraryMediaKind.cs @@ -1,4 +1,4 @@ -namespace ErsatzTV.Core.Domain; +namespace ErsatzTV.Core.Domain; public enum LibraryMediaKind { @@ -8,5 +8,13 @@ public enum LibraryMediaKind OtherVideos = 4, Songs = 5, Images = 6, - RemoteStreams = 7 + RemoteStreams = 7, + + /// + /// A library whose contents are heterogeneous - movies, shows and music videos together. + /// Only produced for remote (Jellyfin) libraries whose collection type is "mixed", where the + /// media server classifies each item for us. A local library is never Mixed: the local folder + /// scanners all share one video extension list and would claim each other's files. + /// + Mixed = 8 } diff --git a/ErsatzTV.Infrastructure.Tests/Jellyfin/JellyfinApiClientTests.cs b/ErsatzTV.Infrastructure.Tests/Jellyfin/JellyfinApiClientTests.cs index a369015da..2dc74150d 100644 --- a/ErsatzTV.Infrastructure.Tests/Jellyfin/JellyfinApiClientTests.cs +++ b/ErsatzTV.Infrastructure.Tests/Jellyfin/JellyfinApiClientTests.cs @@ -53,6 +53,141 @@ public class JellyfinApiClientTests libraries[0].ShouldSyncItems.ShouldBeFalse(); libraries[0].Paths.Single().Path.ShouldBe("jellyfin://library-1"); } + + [Test] + public async Task Should_Project_Mixed_Libraries() + { + const string response = """ + [ + { + "Name": "Music Videos", + "CollectionType": "mixed", + "ItemId": "library-9", + "LibraryOptions": { + "PathInfos": [] + } + } + ] + """; + + var client = new JellyfinApiClient( + new MemoryCache(new MemoryCacheOptions()), + Substitute.For(), + Substitute.For(), + new SingleResponseHttpClientFactory(response), + Substitute.For>()); + + Either> result = + await client.GetLibraries("http://jellyfin.example", "MediaBrowser Token=abc"); + + result.IsRight.ShouldBeTrue(); + List libraries = result.RightToSeq().Single(); + libraries.Count.ShouldBe(1); + libraries[0].Name.ShouldBe("Music Videos"); + libraries[0].ItemId.ShouldBe("library-9"); + libraries[0].MediaKind.ShouldBe(LibraryMediaKind.Mixed); + libraries[0].ShouldSyncItems.ShouldBeFalse(); + libraries[0].Paths.Single().Path.ShouldBe("jellyfin://library-9"); + } + + [Test] + public async Task Should_Project_Libraries_With_No_CollectionType_As_Mixed() + { + const string response = """ + [ + { + "Name": "Standup", + "ItemId": "library-10", + "LibraryOptions": { + "PathInfos": [] + } + } + ] + """; + + var client = new JellyfinApiClient( + new MemoryCache(new MemoryCacheOptions()), + Substitute.For(), + Substitute.For(), + new SingleResponseHttpClientFactory(response), + Substitute.For>()); + + Either> result = + await client.GetLibraries("http://jellyfin.example", "MediaBrowser Token=abc"); + + result.IsRight.ShouldBeTrue(); + List libraries = result.RightToSeq().Single(); + libraries.Count.ShouldBe(1); + libraries[0].Name.ShouldBe("Standup"); + libraries[0].MediaKind.ShouldBe(LibraryMediaKind.Mixed); + } + + // Jellyfin serializes "no content type" as absent, empty or whitespace depending on version; + // all three mean mixed content, so all three must project identically. + [TestCase("\"CollectionType\": \"\",")] + [TestCase("\"CollectionType\": \" \",")] + public async Task Should_Project_Libraries_With_Blank_CollectionType_As_Mixed(string collectionTypeLine) + { + string response = $$""" + [ + { + "Name": "Standup", + {{collectionTypeLine}} + "ItemId": "library-12", + "LibraryOptions": { + "PathInfos": [] + } + } + ] + """; + + var client = new JellyfinApiClient( + new MemoryCache(new MemoryCacheOptions()), + Substitute.For(), + Substitute.For(), + new SingleResponseHttpClientFactory(response), + Substitute.For>()); + + Either> result = + await client.GetLibraries("http://jellyfin.example", "MediaBrowser Token=abc"); + + result.IsRight.ShouldBeTrue(); + List libraries = result.RightToSeq().Single(); + libraries.Count.ShouldBe(1); + libraries[0].MediaKind.ShouldBe(LibraryMediaKind.Mixed); + } + + // Guard: mixed must not become a catch-all. Jellyfin "music" (audio) libraries have no + // supported scanner, so they must keep falling through to None. + [Test] + public async Task Should_Not_Project_Unknown_CollectionTypes() + { + const string response = """ + [ + { + "Name": "Explo Discovery", + "CollectionType": "music", + "ItemId": "library-11", + "LibraryOptions": { + "PathInfos": [] + } + } + ] + """; + + var client = new JellyfinApiClient( + new MemoryCache(new MemoryCacheOptions()), + Substitute.For(), + Substitute.For(), + new SingleResponseHttpClientFactory(response), + Substitute.For>()); + + Either> result = + await client.GetLibraries("http://jellyfin.example", "MediaBrowser Token=abc"); + + result.IsRight.ShouldBeTrue(); + result.RightToSeq().Single().ShouldBeEmpty(); + } } private sealed class SingleResponseHttpClientFactory(string response) : IHttpClientFactory diff --git a/ErsatzTV.Infrastructure/Jellyfin/JellyfinApiClient.cs b/ErsatzTV.Infrastructure/Jellyfin/JellyfinApiClient.cs index 5df85c91d..55930f11c 100644 --- a/ErsatzTV.Infrastructure/Jellyfin/JellyfinApiClient.cs +++ b/ErsatzTV.Infrastructure/Jellyfin/JellyfinApiClient.cs @@ -1,4 +1,4 @@ -using System.Globalization; +using System.Globalization; using ErsatzTV.Core; using ErsatzTV.Core.Domain; using ErsatzTV.Core.Interfaces.Jellyfin; @@ -435,7 +435,9 @@ public class JellyfinApiClient : IJellyfinApiClient } private Option Project(JellyfinLibraryResponse response) => - response.CollectionType?.ToLowerInvariant() switch + // normalize "no content type" to null: Jellyfin serializes a mixed library's collection type + // as absent, empty or whitespace depending on server version, and all three mean the same thing + (string.IsNullOrWhiteSpace(response.CollectionType) ? null : response.CollectionType.ToLowerInvariant()) switch { "tvshows" => new JellyfinLibrary { @@ -466,6 +468,21 @@ public class JellyfinApiClient : IJellyfinApiClient }, // TODO: ??? for music libraries "boxsets" => CacheCollectionLibraryId(response.ItemId), + + // A "mixed content" library. Jellyfin reports these as either the literal "mixed" or with + // no collection type at all, depending on server version. Its items are read per type via + // includeItemTypes, so the mix is resolved authoritatively by Jellyfin rather than guessed. + "mixed" or null => new JellyfinLibrary + { + ItemId = response.ItemId, + Name = response.Name, + MediaKind = LibraryMediaKind.Mixed, + ShouldSyncItems = false, + Paths = new List { new() { Path = $"jellyfin://{response.ItemId}" } }, + PathInfos = GetPathInfos(response) + }, + + // anything else (notably "music" audio libraries) stays unsupported _ => None }; diff --git a/ErsatzTV.Scanner.Tests/Application/Jellyfin/SynchronizeJellyfinLibraryByIdHandlerTests.cs b/ErsatzTV.Scanner.Tests/Application/Jellyfin/SynchronizeJellyfinLibraryByIdHandlerTests.cs index f07ad772f..fbce38d62 100644 --- a/ErsatzTV.Scanner.Tests/Application/Jellyfin/SynchronizeJellyfinLibraryByIdHandlerTests.cs +++ b/ErsatzTV.Scanner.Tests/Application/Jellyfin/SynchronizeJellyfinLibraryByIdHandlerTests.cs @@ -165,14 +165,276 @@ public class SynchronizeJellyfinLibraryByIdHandlerTests // a user-initiated cancellation is not a failure and must not be logged at ERROR (#410) await libraryRepository.DidNotReceive().UpdateLastScan(Arg.Any()); + // prefix match, matching the mixed-library test below: equality would still be exact for + // this single-kind path, but a prefix cannot be quietly defeated by a reworded error logger.ReceivedCalls() .Any(call => call.GetArguments().ElementAtOrDefault(2)?.ToString() - == "Error synchronizing jellyfin library: Scan was canceled") + ?.StartsWith("Error synchronizing jellyfin library:", StringComparison.Ordinal) == true) .ShouldBeFalse(); logger.ReceivedCalls() .Any(call => call.GetArguments().ElementAtOrDefault(2)?.ToString() == "Scan of jellyfin library Concerts was canceled") .ShouldBeTrue(); } + + [Test] + public async Task Should_Scan_All_Three_Kinds_For_Mixed_Libraries() + { + var scannerProxy = Substitute.For(); + var mediaSourceRepository = Substitute.For(); + var jellyfinSecretStore = Substitute.For(); + var jellyfinMovieLibraryScanner = Substitute.For(); + var jellyfinTelevisionLibraryScanner = Substitute.For(); + var jellyfinMusicVideoLibraryScanner = Substitute.For(); + var libraryRepository = Substitute.For(); + var configElementRepository = Substitute.For(); + + var library = new JellyfinLibrary + { + Id = 42, + Name = "Music Videos", + MediaKind = LibraryMediaKind.Mixed, + MediaSourceId = 7 + }; + var mediaSource = new JellyfinMediaSource + { + Id = 7, + Connections = + [ + new JellyfinConnection + { + Address = "http://jellyfin.example", + JellyfinMediaSourceId = 7 + } + ] + }; + + mediaSourceRepository.GetJellyfinByLibraryId(library.Id).Returns(Some(mediaSource).AsTask()); + mediaSourceRepository.GetJellyfinLibrary(library.Id).Returns(Some(library).AsTask()); + jellyfinSecretStore.ReadSecrets().Returns(new JellyfinSecrets + { + Address = "http://jellyfin.example", + ApiKey = "abc" + }); + configElementRepository.GetValue( + Arg.Is(key => key.Key == ConfigElementKey.LibraryRefreshInterval.Key), + Arg.Any()) + .Returns(Task.FromResult>(Some(0))); + + jellyfinMovieLibraryScanner.ScanLibrary( + Arg.Any(), library, true, Arg.Any()) + .Returns(Right(Unit.Default).AsTask()); + jellyfinTelevisionLibraryScanner.ScanLibrary( + Arg.Any(), library, true, Arg.Any()) + .Returns(Right(Unit.Default).AsTask()); + jellyfinMusicVideoLibraryScanner.ScanLibrary( + Arg.Any(), library, true, Arg.Any()) + .Returns(Right(Unit.Default).AsTask()); + + var handler = new SynchronizeJellyfinLibraryByIdHandler( + scannerProxy, + mediaSourceRepository, + jellyfinSecretStore, + jellyfinMovieLibraryScanner, + jellyfinTelevisionLibraryScanner, + jellyfinMusicVideoLibraryScanner, + libraryRepository, + configElementRepository, + Substitute.For>()); + + Either result = await handler.Handle( + new SynchronizeJellyfinLibraryById("http://ersatztv.example", library.Id, true, true), + CancellationToken.None); + + result.LeftToSeq().ShouldBeEmpty(); + result.IsRight.ShouldBeTrue(); + result.RightToSeq().Single().ShouldBe("Music Videos"); + + await jellyfinMovieLibraryScanner.Received(1).ScanLibrary( + Arg.Any(), library, true, Arg.Any()); + await jellyfinTelevisionLibraryScanner.Received(1).ScanLibrary( + Arg.Any(), library, true, Arg.Any()); + await jellyfinMusicVideoLibraryScanner.Received(1).ScanLibrary( + Arg.Any(), library, true, Arg.Any()); + await libraryRepository.Received(1).UpdateLastScan(library); + } + + [Test] + public async Task Should_Run_Remaining_Scanners_When_One_Fails_For_Mixed_Libraries() + { + var scannerProxy = Substitute.For(); + var mediaSourceRepository = Substitute.For(); + var jellyfinSecretStore = Substitute.For(); + var jellyfinMovieLibraryScanner = Substitute.For(); + var jellyfinTelevisionLibraryScanner = Substitute.For(); + var jellyfinMusicVideoLibraryScanner = Substitute.For(); + var libraryRepository = Substitute.For(); + var configElementRepository = Substitute.For(); + + var library = new JellyfinLibrary + { + Id = 42, + Name = "Music Videos", + MediaKind = LibraryMediaKind.Mixed, + MediaSourceId = 7 + }; + var mediaSource = new JellyfinMediaSource + { + Id = 7, + Connections = + [ + new JellyfinConnection + { + Address = "http://jellyfin.example", + JellyfinMediaSourceId = 7 + } + ] + }; + + mediaSourceRepository.GetJellyfinByLibraryId(library.Id).Returns(Some(mediaSource).AsTask()); + mediaSourceRepository.GetJellyfinLibrary(library.Id).Returns(Some(library).AsTask()); + jellyfinSecretStore.ReadSecrets().Returns(new JellyfinSecrets + { + Address = "http://jellyfin.example", + ApiKey = "abc" + }); + configElementRepository.GetValue( + Arg.Is(key => key.Key == ConfigElementKey.LibraryRefreshInterval.Key), + Arg.Any()) + .Returns(Task.FromResult>(Some(0))); + + jellyfinMovieLibraryScanner.ScanLibrary( + Arg.Any(), library, true, Arg.Any()) + .Returns(Left(BaseError.New("movie scan blew up")).AsTask()); + jellyfinTelevisionLibraryScanner.ScanLibrary( + Arg.Any(), library, true, Arg.Any()) + .Returns(Right(Unit.Default).AsTask()); + jellyfinMusicVideoLibraryScanner.ScanLibrary( + Arg.Any(), library, true, Arg.Any()) + .Returns(Right(Unit.Default).AsTask()); + + var handler = new SynchronizeJellyfinLibraryByIdHandler( + scannerProxy, + mediaSourceRepository, + jellyfinSecretStore, + jellyfinMovieLibraryScanner, + jellyfinTelevisionLibraryScanner, + jellyfinMusicVideoLibraryScanner, + libraryRepository, + configElementRepository, + Substitute.For>()); + + Either result = await handler.Handle( + new SynchronizeJellyfinLibraryById("http://ersatztv.example", library.Id, true, true), + CancellationToken.None); + + // the movie arm failed, so the library as a whole failed... + result.IsLeft.ShouldBeTrue(); + + // ...but the other two kinds were still ingested + await jellyfinTelevisionLibraryScanner.Received(1).ScanLibrary( + Arg.Any(), library, true, Arg.Any()); + await jellyfinMusicVideoLibraryScanner.Received(1).ScanLibrary( + Arg.Any(), library, true, Arg.Any()); + + // and LastScan is not stamped, because the scan was not fully successful + await libraryRepository.DidNotReceive().UpdateLastScan(Arg.Any()); + } + + [Test] + public async Task Should_Stop_Scanning_Mixed_Library_When_Canceled() + { + var scannerProxy = Substitute.For(); + var mediaSourceRepository = Substitute.For(); + var jellyfinSecretStore = Substitute.For(); + var jellyfinMovieLibraryScanner = Substitute.For(); + var jellyfinTelevisionLibraryScanner = Substitute.For(); + var jellyfinMusicVideoLibraryScanner = Substitute.For(); + var libraryRepository = Substitute.For(); + var configElementRepository = Substitute.For(); + + var library = new JellyfinLibrary + { + Id = 42, + Name = "Music Videos", + MediaKind = LibraryMediaKind.Mixed, + MediaSourceId = 7 + }; + var mediaSource = new JellyfinMediaSource + { + Id = 7, + Connections = + [ + new JellyfinConnection + { + Address = "http://jellyfin.example", + JellyfinMediaSourceId = 7 + } + ] + }; + + mediaSourceRepository.GetJellyfinByLibraryId(library.Id).Returns(Some(mediaSource).AsTask()); + mediaSourceRepository.GetJellyfinLibrary(library.Id).Returns(Some(library).AsTask()); + jellyfinSecretStore.ReadSecrets().Returns(new JellyfinSecrets + { + Address = "http://jellyfin.example", + ApiKey = "abc" + }); + configElementRepository.GetValue( + Arg.Is(key => key.Key == ConfigElementKey.LibraryRefreshInterval.Key), + Arg.Any()) + .Returns(Task.FromResult>(Some(0))); + + var logger = Substitute.For>(); + + jellyfinMovieLibraryScanner.ScanLibrary( + Arg.Any(), library, true, Arg.Any()) + .Returns(Left(new ScanCanceled()).AsTask()); + + var handler = new SynchronizeJellyfinLibraryByIdHandler( + scannerProxy, + mediaSourceRepository, + jellyfinSecretStore, + jellyfinMovieLibraryScanner, + jellyfinTelevisionLibraryScanner, + jellyfinMusicVideoLibraryScanner, + libraryRepository, + configElementRepository, + logger); + + Either result = await handler.Handle( + new SynchronizeJellyfinLibraryById("http://ersatztv.example", library.Id, true, true), + CancellationToken.None); + + result.IsLeft.ShouldBeTrue(); + + // a user-initiated cancellation aborts the whole library; later kinds must not run + await jellyfinTelevisionLibraryScanner.DidNotReceive().ScanLibrary( + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any()); + await jellyfinMusicVideoLibraryScanner.DidNotReceive().ScanLibrary( + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any()); + await libraryRepository.DidNotReceive().UpdateLastScan(Arg.Any()); + + // ScanMixedLibrary must return the ScanCanceled INSTANCE unwrapped, not folded into an + // aggregate BaseError -- otherwise `error is ScanCanceled` in the caller fails and a user + // cancellation is demoted to an ERROR log (#410). + logger.ReceivedCalls() + .Any(call => call.GetArguments().ElementAtOrDefault(2)?.ToString() + == "Scan of jellyfin library Music Videos was canceled") + .ShouldBeTrue(); + // prefix match, not equality: if ScanCanceled were folded into the aggregate error the + // rendered message becomes "...: Mixed library X had 1 scan error(s): Scan was canceled", + // which an equality assertion would NOT catch -- making the check vacuous + logger.ReceivedCalls() + .Any(call => call.GetArguments().ElementAtOrDefault(2)?.ToString() + ?.StartsWith("Error synchronizing jellyfin library:", StringComparison.Ordinal) == true) + .ShouldBeFalse(); + } } } diff --git a/ErsatzTV.Scanner/Application/Jellyfin/Commands/SynchronizeJellyfinLibraryByIdHandler.cs b/ErsatzTV.Scanner/Application/Jellyfin/Commands/SynchronizeJellyfinLibraryByIdHandler.cs index efe0beb77..f5c480818 100644 --- a/ErsatzTV.Scanner/Application/Jellyfin/Commands/SynchronizeJellyfinLibraryByIdHandler.cs +++ b/ErsatzTV.Scanner/Application/Jellyfin/Commands/SynchronizeJellyfinLibraryByIdHandler.cs @@ -85,7 +85,10 @@ public class parameters.Library, parameters.DeepScan, cancellationToken), - _ => Unit.Default + LibraryMediaKind.Mixed => + await ScanMixedLibrary(parameters, cancellationToken), + _ => BaseError.New( + $"Jellyfin library {parameters.Library.Name} has unsupported media kind {parameters.Library.MediaKind}") }; if (result.IsRight) @@ -116,6 +119,73 @@ public class return parameters.Library.Name; } + /// + /// Scans a mixed-content library by running each per-kind scanner against it in turn. Jellyfin + /// resolves the mix server-side - each scanner queries with its own includeItemTypes - so the + /// passes see disjoint item sets, and their reconciliation is type-scoped and cannot + /// cross-delete. + /// + private async Task> ScanMixedLibrary( + RequestParameters parameters, + CancellationToken cancellationToken) + { + _logger.LogInformation( + "Scanning mixed-content jellyfin library {Name}", + parameters.Library.Name); + + var scans = new Func>>[] + { + () => _jellyfinMovieLibraryScanner.ScanLibrary( + parameters.ConnectionParameters, + parameters.Library, + parameters.DeepScan, + cancellationToken), + () => _jellyfinTelevisionLibraryScanner.ScanLibrary( + parameters.ConnectionParameters, + parameters.Library, + parameters.DeepScan, + cancellationToken), + () => _jellyfinMusicVideoLibraryScanner.ScanLibrary( + parameters.ConnectionParameters, + parameters.Library, + parameters.DeepScan, + cancellationToken) + }; + + var errors = new List(); + + foreach (Func>> scan in scans) + { + Either result = await scan(); + + foreach (BaseError error in result.LeftToSeq()) + { + // a cancellation aborts the whole library immediately; it is not one kind failing + if (error is ScanCanceled) + { + return error; + } + + // one kind failing must not stop the others from being ingested + _logger.LogWarning( + "Error scanning one media kind of mixed jellyfin library {Name}: {Error}", + parameters.Library.Name, + error.Value); + + errors.Add(error); + } + } + + if (errors.Count > 0) + { + return BaseError.New( + $"Mixed library {parameters.Library.Name} had {errors.Count} scan error(s): " + + string.Join("; ", errors.Map(e => e.Value))); + } + + return Unit.Default; + } + private async Task> Validate( SynchronizeJellyfinLibraryById request, CancellationToken cancellationToken) => diff --git a/ErsatzTV.Scanner/Application/Jellyfin/Commands/SynchronizeJellyfinShowByIdHandler.cs b/ErsatzTV.Scanner/Application/Jellyfin/Commands/SynchronizeJellyfinShowByIdHandler.cs index b40467b6f..27c2de30f 100644 --- a/ErsatzTV.Scanner/Application/Jellyfin/Commands/SynchronizeJellyfinShowByIdHandler.cs +++ b/ErsatzTV.Scanner/Application/Jellyfin/Commands/SynchronizeJellyfinShowByIdHandler.cs @@ -48,7 +48,8 @@ public class RequestParameters parameters, CancellationToken cancellationToken) { - if (parameters.Library.MediaKind != LibraryMediaKind.Shows) + // a mixed library legitimately contains shows alongside movies and music videos + if (parameters.Library.MediaKind is not (LibraryMediaKind.Shows or LibraryMediaKind.Mixed)) { return BaseError.New($"Library {parameters.Library.Name} is not a TV show library"); } diff --git a/ErsatzTV.Scanner/Application/MediaSources/Commands/ScanLocalLibraryHandler.cs b/ErsatzTV.Scanner/Application/MediaSources/Commands/ScanLocalLibraryHandler.cs index 23a992d91..26f7bf8ec 100644 --- a/ErsatzTV.Scanner/Application/MediaSources/Commands/ScanLocalLibraryHandler.cs +++ b/ErsatzTV.Scanner/Application/MediaSources/Commands/ScanLocalLibraryHandler.cs @@ -139,7 +139,11 @@ public class ScanLocalLibraryHandler : IRequestHandler Unit.Default + + // returning success here would stamp LastScan as though the library had been + // scanned; a local library has no scanner for Mixed and never should + _ => BaseError.New( + $"Local library {localLibrary.Name} has unsupported media kind {localLibrary.MediaKind}") }; if (result.IsRight) diff --git a/ErsatzTV.Tests/Application/Libraries/CreateLocalLibraryHandlerTests.cs b/ErsatzTV.Tests/Application/Libraries/CreateLocalLibraryHandlerTests.cs index 47d2b4a58..3a11f664a 100644 --- a/ErsatzTV.Tests/Application/Libraries/CreateLocalLibraryHandlerTests.cs +++ b/ErsatzTV.Tests/Application/Libraries/CreateLocalLibraryHandlerTests.cs @@ -42,6 +42,28 @@ public class CreateLocalLibraryHandlerTests result.IfLeft(error => error.Value.ShouldContain("/media/movies")); } + // LibraryMediaKind.Mixed exists only for remote (Jellyfin) libraries, where the media server + // classifies each item. No local folder scanner handles it, so a local Mixed library would fail + // every scan forever and log at ERROR on every scheduler tick. The API takes a raw + // LibraryMediaKind, so hiding it from the SPA dropdown is not enforcement (#489 review M1). + [Test] + public async Task Handle_Should_Reject_The_Mixed_Media_Kind_For_Local_Libraries() + { + await SeedLocalMediaSource(); + + var fileSystem = new MockFileSystem(); + fileSystem.Directory.CreateDirectory("/media/music"); + + CreateLocalLibraryHandler handler = CreateHandler(fileSystem); + + Either result = await handler.Handle( + new CreateLocalLibrary("Music", LibraryMediaKind.Mixed, ["/media/music"]), + CancellationToken.None); + + result.IsLeft.ShouldBeTrue(); + result.IfLeft(error => error.Value.ShouldContain("Mixed")); + } + [Test] public async Task Handle_Should_List_Only_The_Missing_Paths_When_Mixed() { diff --git a/ErsatzTV/Controllers/Api/LocalLibrariesController.cs b/ErsatzTV/Controllers/Api/LocalLibrariesController.cs index d6ad63d31..6b44a7f6c 100644 --- a/ErsatzTV/Controllers/Api/LocalLibrariesController.cs +++ b/ErsatzTV/Controllers/Api/LocalLibrariesController.cs @@ -55,6 +55,10 @@ public class LocalLibrariesController( [HttpPost("/api/v1/libraries/local")] [Tags("Libraries")] [EndpointSummary("Create a local library")] + [EndpointDescription( + "The shared LibraryMediaKind enum includes Mixed, but it is rejected here with 422. Mixed exists " + + "only for Jellyfin libraries, where the media server classifies each item; no local folder " + + "scanner handles it.")] [ProducesResponseType(typeof(LocalLibraryResponseModel), StatusCodes.Status201Created)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task Create( diff --git a/ErsatzTV/wwwroot/openapi/v1.json b/ErsatzTV/wwwroot/openapi/v1.json index b168e4324..e0c8ec4d3 100644 --- a/ErsatzTV/wwwroot/openapi/v1.json +++ b/ErsatzTV/wwwroot/openapi/v1.json @@ -9514,6 +9514,7 @@ "Libraries" ], "summary": "Create a local library", + "description": "The shared LibraryMediaKind enum includes Mixed, but it is rejected here with 422. Mixed exists only for Jellyfin libraries, where the media server classifies each item; no local folder scanner handles it.", "operationId": "LocalLibrariesCreate", "requestBody": { "content": { @@ -27011,7 +27012,8 @@ "OtherVideos", "Songs", "Images", - "RemoteStreams" + "RemoteStreams", + "Mixed" ], "type": "string" }, diff --git a/docs/decisions.md b/docs/decisions.md index e89069f2c..a6208807f 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -897,6 +897,7 @@ relabelled to reflect the #295 reality. Convention + hook documented in `spa-con implicit route-label ⟺ screen-subscription coupling is the kind of thing #247 (shell extraction) will formalize; #238 keeps it a documented convention guarded by a data-driven `App.test.tsx` test over the URL-navigating create screens (a typo'd route id → the banner navigates nowhere → red). Refs #238 #247. + ## 2026-07-13 — API versioning: the whole `/api` surface is mounted at `/api/v1`, additive-only after freeze (#286) The #197 cold review's C1 **BLOCKER**: `/api/*` was entirely unversioned (`info.version` was cosmetic), so the @@ -1214,6 +1215,7 @@ channel" has no reason to look under "Alternate Schedules", and on finding that Fixed with a task-shaped **"Recipe: seasonal / holiday programming"** section in `channels.md` (both engines, plus the gotchas above) and a `domain-model.md` glossary row. No production code changed, so no live-E2E (same reasoning as #77). + ## 2026-07-17 — Auto-Tune DetailPanel member list = live search-index roll-up, not EF enumeration (#384) The Auto-Tune DetailPanel (#383) shows, per proposed channel, the distinct **content sources** its @@ -1357,6 +1359,7 @@ not size the `small` lane expecting otherwise. **Not addressed here**: the 12–35 min queue waits (server-management#604) and the redundant triple-build (#398). + ## 2026-07-17 — Channel health on the API = the raw `PlayoutCount` fact on the list DTO, not a derived status enum (#72) #72 asks for per-channel status in the Channels list, "especially channels that will fail to play". @@ -1400,6 +1403,7 @@ Keeping all three out held #72 to a **read-path-only** change: no migration, no channel editor's playout-source guard. Both call sites now share `Mapper.GetPlayoutsCount` (Mirror-aware). The related *silent* server-side coercion of Mirror→Generated (a 200 that discards the caller's intent, against the §3 "surface it, don't silently filter" rule) is filed as **#401**, not fixed here. + ## 2026-07-17 — Weighted / fair-share distribution is a new `WeightedShuffle` order; `ShuffleInOrder` is anti-clumping, not fair-share (#70) `PlaybackOrder.WeightedShuffle = 9` ships the two behaviors #70 asked for — "air Show A 70% / Show B 30%" @@ -1535,6 +1539,7 @@ part — is skipped. Two adjacent redundancies are deliberately **out of scope**: the whole matrix re-running on a PR and again on the merge-to-`main` over identical code (#420), and the within-run triple `dotnet build` (#398). Full mechanism in `ci-cd.md` → "Docs-only skip". + ## 2026-07-17 — Docs-only detect must be shallow-checkout safe: FETCH_HEAD + two-dot, not origin/main + three-dot (#416 follow-up) The #416 docs-only skip shipped (#422) safe but **ineffective**: every docs-only PR still ran the full @@ -1550,6 +1555,7 @@ because they use `fetch-depth: 0` — a difference the first cut missed. Meta-le gating change can pass every local test and merge green while being a complete no-op in CI; only real-PR verification that **measures the effect** (job durations, not just a green check) catches it — which is exactly what #416's Done-when demanded. Fixed in the #416 follow-up PR. + ## 2026-07-17 — Pre-push guard: don't push a file whose working-tree copy is uncommitted (H13, #416 session) A review fix (`--no-renames`) was edited into the working file and empirically verified there, but a @@ -1614,6 +1620,7 @@ shipped here; the rest is deliberately deferred rather than forced. materialization — neither is the additive plumbing this slice was scoped to, and it is a channel-wide capability rather than an auto-tune concern. Left for its own issue / the SPA branding work; the uploaded `logo` above already covers the on-screen bug for channels that supply an image. + ## 2026-07-17 — Health-check remediation is server-declared `{Kind, Target}` on an additive DTO; the SPA acts on it (#164) #164 asked to make the ~14 health checks *actionable* — the Dashboard health panel showed problems @@ -1722,6 +1729,7 @@ index actually supports. for value inputs, relative-date operators, nesting deeper than one level, and inline adoption of `RuleBuilder` by ChannelBuilder / Auto-Tune (it was built reusable for exactly that reuse — see `spa-conventions.md` §12). + ## 2026-07-18 — Auto-Tune per-source weights ride #70's MultiCollection machinery; created at tune time, not a post-hoc PUT (#425) Per-source rotation weights (`3× Show A, 1× Show B`) and query corrections (exclude / add-untagged) for @@ -1889,6 +1897,7 @@ the three still-**silent** build-time dispatch sites — the ones this issue nam `SeasonEpisodeMediaCollectionEnumerator`) that simply aren't reverse-mapped, so making it "loud" would emit false-positive warnings. Completing that reverse map is a separate concern from "an unsupported *order* degrades silently" and is not part of #403's scope. + ## 2026-07-18 — CI build-once was measured and rejected; keep the #420 tree-skip Build-once (a `compile` job producing a single artifact, consumed by `test`/`migrations`/ @@ -2081,6 +2090,7 @@ Disabling the servers is worth it (consistent reduction, no resident server), bu off the live test-job peak-anon this instrument now reports, not off the build-only A/B. The older #411 probe (`anon 7134 MiB`) read higher than these swap-off sampled numbers and is superseded (swap/read-method move the figure >1 GiB). + ## 2026-07-19 — Media-server remote-stream URLs are probed before use: a redirected 404 fails closed, everything else fails open, no toggle (#473) `GetPlayoutItemProcessByChannelNumberHandler.ValidatePlayoutItemPath` now probes the Plex/Jellyfin/Emby @@ -2290,3 +2300,64 @@ media-server identity those base scanners rely on. SQLite, extending the #488 harness) pin removal, empty-artist cleanup, cross-kind safety, and the empty-fetch guard. Proven non-vacuous: all four fail against the pre-fix scanner except the guard control, which only earns its keep once the sweep exists. + +## 2026-07-20 (#489) — Jellyfin mixed-content libraries map to one library holding many kinds + +A Jellyfin library whose collection type is `mixed` — or absent — now maps to `LibraryMediaKind.Mixed` +instead of being dropped by `JellyfinApiClient.Project()`'s `_ => None`. Scanning it runs the movie, +television and music-video scanners in sequence against that one library. + +- **A library is a place, not a media kind.** One physical path ↔ one Jellyfin library ↔ one ErsatzTV + library, whose contents are heterogeneous. This is what keeps music and standup content segregated from + the main `Movies` and `TV Shows` libraries, which was the actual goal (#474). The previous workaround was + a *local* library pointed at the same tree, which bypassed Jellyfin, scanned the content twice and + mis-modelled shows as movies. +- **The classification is Jellyfin's, not ours.** Each scanner queries `parentId` + `includeItemTypes` + (`"Movie"` / `"Series"` / `"MusicVideo"`), so the three passes receive disjoint, authoritative sets. + Nothing is inferred from folder shape or NFO contents. This is why mixed support is tractable at all — an + earlier reading that it would require guessing per item was wrong. +- **No migration.** `MediaItem` is table-per-type with no discriminator and `LibraryPathId` on the abstract + base, so heterogeneous items under one `LibraryPath` were always legal; + `MediaItemRepository.GetAllTrashedItems` already `COALESCE`s across every subclass id for a single path. +- **No cross-deletion.** Reconciliation is type-scoped (`GetExistingMovies(library)` and friends), so a pass + over one kind cannot trash another kind's items in the same library. +- **`MediaKind` is dispatch + presentation only.** Scheduling, playout, collections, smart collections and + search hold zero references to it; search keys off the item's own subclass and the SPA's browse screens + use a per-item `LibraryBrowseMediaType`. Adding a kind is therefore cheap. + +**Deliberately scoped to Jellyfin. Local mixed libraries are NOT supported** and `Mixed` is absent from the +SPA's local-library media-kind options. Locally the same approach is unsafe: every local scanner shares +`LocalFolderScanner.VideoFileExtensions`, so the movie scanner would claim episode files, and `LibraryFolder` +rows are keyed by `LibraryPathId` with no notion of kind, so two scanners over one path would thrash each +other's etags. Neither hazard exists remotely — among the remote scanners only +`JellyfinMusicVideoLibraryScanner` touches `LibraryFolder` at all. (The existing `Images` + `OtherVideos` +shared-path exemption in `LocalLibraryHandlerBase.AreSubPaths` is the closest local analogue, and it already +carries that etag contention.) + +**Failure semantics of the `Mixed` arm**: one kind failing does *not* skip the others — a broken music-video +scan must not prevent the movies in the same library from being ingested — but the library as a whole then +reports failure and `LastScan` is not stamped. `ScanCanceled` aborts the sequence immediately, since a user +cancellation is not one kind failing. + +**Also fixed here**: `ScanLocalLibraryHandler` and `SynchronizeJellyfinLibraryByIdHandler` both ended their +dispatch switch with `_ => Unit.Default`, returning **success** for an unhandled kind and stamping `LastScan` +as though a scan had run. Both now return a `BaseError`. That silent success is precisely how a missing +`Mixed` arm would have hidden, so removing it is part of the feature, not a drive-by. + +**Known limitations of the `Mixed` arm**, surfaced by the adversarial review and accepted rather than +fixed here: + +- **Scan progress resets twice.** Each per-kind scanner independently drives `_scannerProxy.UpdateProgress` + from 0 to 1 over its own item set, so a mixed library's progress bar fills and resets three times. The + local scanner solves this by threading `progressMin`/`progressMax` per path (`ScanLocalLibraryHandler`), + but the Jellyfin `ScanLibrary` signature has no such parameter, so fixing it means changing three scanner + interfaces. Cosmetic, and deliberately out of scope. +- **One permanently-failing kind forces the healthy kinds to rescan forever.** `ScanMixedLibrary` returns + `Left` if any arm failed, so `LastScan` is never stamped and the whole library re-scans every interval. + Single-kind libraries already behave this way; `Mixed` widens the blast radius to the other two kinds. + Accepted: the alternative — stamping `LastScan` on partial success — would silently mask a broken kind, + which is worse. +- **`JellyfinMusicVideoLibraryScanner` performs no reconciliation at all** — no `GetExisting*`, no trash + sweep. This is *why* it cannot cross-delete in a mixed library, but it also means a music video removed + from Jellyfin is never removed from ErsatzTV. Pre-existing and orthogonal to this change; it belongs with + that scanner's other gaps (no `ItemId`/`Etag`, path-keyed identity — see #488). diff --git a/web/src/api/generated/v1.d.ts b/web/src/api/generated/v1.d.ts index 5d5b8511e..c05d1c554 100644 --- a/web/src/api/generated/v1.d.ts +++ b/web/src/api/generated/v1.d.ts @@ -813,7 +813,7 @@ export interface components { "seasonId"?: null | number; }; "LibraryBrowseMediaType": "Movie" | "TelevisionShow" | "TelevisionSeason" | "Artist" | "Collection" | "SmartCollection" | "MultiCollection" | "RerunCollection" | "Playlist" | "Episode" | "MusicVideo" | "Song" | "OtherVideo" | "Image" | "RemoteStream"; - "LibraryMediaKind": "Movies" | "Shows" | "MusicVideos" | "OtherVideos" | "Songs" | "Images" | "RemoteStreams"; + "LibraryMediaKind": "Movies" | "Shows" | "MusicVideos" | "OtherVideos" | "Songs" | "Images" | "RemoteStreams" | "Mixed"; "LibraryScanStatusResponseModel": { "libraryId": number; "percent": number; diff --git a/web/src/screens/LibrariesScreen.test.tsx b/web/src/screens/LibrariesScreen.test.tsx index 23d8c34e1..8218d50f5 100644 --- a/web/src/screens/LibrariesScreen.test.tsx +++ b/web/src/screens/LibrariesScreen.test.tsx @@ -283,13 +283,28 @@ describe('LibrariesScreen', () => { // few microtask ticks after runPollTick's `act(async () => ...)` callback has already resolved. // Asserting synchronously right after the tick races that chain (#447); waitFor lets both // settle before asserting, without weakening either check. - await waitFor(() => { - expect(screen.queryByText('75%')).not.toBeInTheDocument(); - }); - await waitFor(() => { - expect(fetchCount(handle, '/api/v1/media-sources')).toBe(sourceFetchesBeforeCompletion + 1); - }); - }); + // waitFor's 1000ms default is ample locally (this file runs in ~1s) but marginal on the CI VM, + // where the SPA suite runs ~8x slower under load — observed failing there on a types-only diff + // that cannot affect this screen. The per-waitFor timeout must stay under the test-level budget + // below, or the test dies on vitest's timeout with no assertion diff instead of waitFor's. + await waitFor( + () => { + expect(screen.queryByText('75%')).not.toBeInTheDocument(); + }, + { timeout: 5000 } + ); + await waitFor( + () => { + expect(fetchCount(handle, '/api/v1/media-sources')).toBe(sourceFetchesBeforeCompletion + 1); + }, + { timeout: 5000 } + ); + // Generous test timeout: vitest defaults to 5s for the whole test, which the two waits above + // would otherwise share with render + findByText + runPollTick. Budgeted so the waits cannot + // exhaust it: 2 x 5s of waits plus a CI-slowed prelude still leaves headroom at 20s, so a + // failure is reported by waitFor (with an assertion diff) rather than by vitest's generic + // timeout. Same reason as TrashScreen.test.tsx (run 686) and CollectionsScreen.test.tsx. + }, 20000); it('reconciles a 409 "already scanning" against scan-status without an error toast (#232)', async () => { mockApi({