From 4a9502cbb5b2a82384e06a5cff73978afd4f2acf Mon Sep 17 00:00:00 2001 From: Timothy Date: Mon, 20 Jul 2026 00:17:15 +0200 Subject: [PATCH] fix(480): probe external-JSON remote-stream URLs before handing them to ffmpeg External-JSON playout channels build their own /media/plex/{server}/{plexFile} URL in ExternalJsonPlayoutItemProvider.StreamRemotely and the handler assigns it without routing through ValidatePlayoutItemPath, so the #473 class survived here: a media item gone from the server 404s under ffmpeg (exit 8) and the same dead item is re-selected for its whole slot. Route StreamRemotely through the same IRemoteStreamProber seam #473/PR #479 added for the generated-playout path. Probe runs before the Plex metadata round-trip (the URL needs only server.Id + plexFile), so a gone item skips it. An unavailable stream returns PlayoutItemNotAvailableFromMediaServer, which the handler already maps to a real-error card. The fail-open policy (redirected-404 only) lives inside IRemoteStreamProber, so this second call site duplicates only the decision to probe. Tests: ExternalJsonPlayoutItemProviderTests pins both directions; proven non-vacuous by neutralizing the probe. docs/decisions.md gets a #480 entry closing the #473 scope gap (append-only: the old #473 entry is cross-referenced, not edited). fixes #480 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ExternalJsonPlayoutItemProvider.cs | 23 +- .../ExternalJsonPlayoutItemProviderTests.cs | 220 ++++++++++++++++++ docs/decisions.md | 35 +++ 3 files changed, 275 insertions(+), 3 deletions(-) create mode 100644 ErsatzTV.Tests/Infrastructure/Streaming/ExternalJsonPlayoutItemProviderTests.cs diff --git a/ErsatzTV.Infrastructure/Streaming/ExternalJsonPlayoutItemProvider.cs b/ErsatzTV.Infrastructure/Streaming/ExternalJsonPlayoutItemProvider.cs index 7e73234c4..f06f45698 100644 --- a/ErsatzTV.Infrastructure/Streaming/ExternalJsonPlayoutItemProvider.cs +++ b/ErsatzTV.Infrastructure/Streaming/ExternalJsonPlayoutItemProvider.cs @@ -26,6 +26,7 @@ public class ExternalJsonPlayoutItemProvider : IExternalJsonPlayoutItemProvider private readonly IPlexPathReplacementService _plexPathReplacementService; private readonly IPlexSecretStore _plexSecretStore; private readonly IPlexServerApiClient _plexServerApiClient; + private readonly IRemoteStreamProber _remoteStreamProber; public ExternalJsonPlayoutItemProvider( IDbContextFactory dbContextFactory, @@ -34,6 +35,7 @@ public class ExternalJsonPlayoutItemProvider : IExternalJsonPlayoutItemProvider IPlexServerApiClient plexServerApiClient, IPlexSecretStore plexSecretStore, ILocalStatisticsProvider localStatisticsProvider, + IRemoteStreamProber remoteStreamProber, ILogger logger) { _dbContextFactory = dbContextFactory; @@ -42,6 +44,7 @@ public class ExternalJsonPlayoutItemProvider : IExternalJsonPlayoutItemProvider _plexServerApiClient = plexServerApiClient; _plexSecretStore = plexSecretStore; _localStatisticsProvider = localStatisticsProvider; + _remoteStreamProber = remoteStreamProber; _logger = logger; } @@ -217,15 +220,29 @@ public class ExternalJsonPlayoutItemProvider : IExternalJsonPlayoutItemProvider foreach (PlexServerAuthToken token in maybeToken) { + var plexUrl = + $"http://localhost:{Settings.StreamingPort}/media/plex/{server.Id}/{program.PlexFile}"; + + // #480: probe the remote-stream URL before handing it to ffmpeg, exactly as the + // generated-playout path does in + // GetPlayoutItemProcessByChannelNumberHandler.ValidatePlayoutItemPath (#473). Without + // this, an item that is gone from the media server 404s under ffmpeg (exit 8) and the + // same dead item is re-selected for its whole slot. The fail-open contract (only a + // *redirected* 404 fails closed) lives inside IRemoteStreamProber, so this call site + // only owns the decision to probe, not the policy. Probing first also skips the Plex + // metadata round-trip when the item is already gone. + if (!await _remoteStreamProber.IsAvailable(plexUrl, cancellationToken)) + { + return new PlayoutItemNotAvailableFromMediaServer(plexUrl); + } + MediaItem mediaItem = program.Type switch { "episode" => await GetPlexEpisode(server, connection, token, program), _ => await GetPlexMovie(server, connection, token, program) }; - return new PlayoutItemWithPath( - GetPlayoutItem(startTime, mediaItem, program), - $"http://localhost:{Settings.StreamingPort}/media/plex/{server.Id}/{program.PlexFile}"); + return new PlayoutItemWithPath(GetPlayoutItem(startTime, mediaItem, program), plexUrl); } } } diff --git a/ErsatzTV.Tests/Infrastructure/Streaming/ExternalJsonPlayoutItemProviderTests.cs b/ErsatzTV.Tests/Infrastructure/Streaming/ExternalJsonPlayoutItemProviderTests.cs new file mode 100644 index 000000000..bdbdacc4e --- /dev/null +++ b/ErsatzTV.Tests/Infrastructure/Streaming/ExternalJsonPlayoutItemProviderTests.cs @@ -0,0 +1,220 @@ +using System.IO.Abstractions; +using ErsatzTV.Core; +using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Errors; +using ErsatzTV.Core.Interfaces.Metadata; +using ErsatzTV.Core.Interfaces.Plex; +using ErsatzTV.Core.Interfaces.Streaming; +using ErsatzTV.Core.Plex; +using ErsatzTV.Core.Streaming; +using ErsatzTV.Infrastructure.Data; +using ErsatzTV.Infrastructure.Streaming; +using ErsatzTV.Tests.Support; +using LanguageExt; +using Newtonsoft.Json; +using NSubstitute; +using NUnit.Framework; +using Shouldly; +using static LanguageExt.Prelude; +using DomainChannel = ErsatzTV.Core.Domain.Channel; + +namespace ErsatzTV.Tests.Infrastructure.Streaming; + +/// +/// ersatztv#480: external-JSON playout channels build their own /media/plex/... URL and used +/// to hand it to ffmpeg unprobed, so the #473 class survived there — a media item gone from the +/// server 404s under ffmpeg (exit 8) and the same dead item is re-selected for its whole slot. The +/// remote path must now probe the URL via first, failing closed +/// only on a media-server (redirected) 404. The fail-open contract itself is pinned by +/// HttpRemoteStreamProberTests; these tests pin that the external-JSON path routes through it. +/// +[TestFixture] +public class ExternalJsonPlayoutItemProviderTests +{ + private const string ServerKey = "server1"; + private const string ClientIdentifier = "client1"; + private const string PlexFile = "shows/example/s01e01.mkv"; + private const string ScheduleFile = "/config/externaljson/channel1.json"; + private const string LocalPath = "/plex/shows/example/s01e01.mkv"; + + private readonly DateTimeOffset _now = new(2026, 1, 1, 12, 0, 0, TimeSpan.Zero); + + private InMemoryTvContext _db = null!; + private IFileSystem _fileSystem = null!; + private ILocalStatisticsProvider _localStatisticsProvider = null!; + private IPlexPathReplacementService _plexPathReplacementService = null!; + private IPlexSecretStore _plexSecretStore = null!; + private IPlexServerApiClient _plexServerApiClient = null!; + private IRemoteStreamProber _remoteStreamProber = null!; + + private DomainChannel _channel = null!; + private int _serverId; + + [SetUp] + public async Task SetUp() + { + _db = await InMemoryTvContext.CreateAsync(); + _fileSystem = Substitute.For(); + _localStatisticsProvider = Substitute.For(); + _plexPathReplacementService = Substitute.For(); + _plexSecretStore = Substitute.For(); + _plexServerApiClient = Substitute.For(); + _remoteStreamProber = Substitute.For(); + + await SeedAsync(); + + // the JSON schedule exists, but the resolved local file does not — so the provider takes the + // remote-stream branch, which is the one #480 fixes + _fileSystem.File.Exists(ScheduleFile).Returns(true); + _fileSystem.File.Exists(LocalPath).Returns(false); + _fileSystem.File.ReadAllTextAsync(ScheduleFile, Arg.Any()).Returns(ScheduleJson()); + + _plexPathReplacementService + .GetReplacementPlexPath(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(LocalPath); + + _plexSecretStore.GetServerAuthToken(Arg.Any()) + .Returns(Option.Some(new PlexServerAuthToken(ClientIdentifier, "token"))); + } + + [TearDown] + public async Task TearDown() => await _db.DisposeAsync(); + + private string ExpectedUrl => $"http://localhost:{Settings.StreamingPort}/media/plex/{_serverId}/{PlexFile}"; + + [Test] + public async Task Should_Return_Not_Available_Error_When_Remote_Stream_Is_Unavailable() + { + _remoteStreamProber.IsAvailable(Arg.Any(), Arg.Any()).Returns(false); + + Either result = + await CreateProvider().CheckForExternalJson(_channel, _now, "/ffprobe", CancellationToken.None); + + // fails closed with the error the handler renders as a real "not available" card (not exit 8) + result.IsLeft.ShouldBeTrue(); + result.LeftToSeq().Head.ShouldBeOfType(); + + await _remoteStreamProber.Received(1).IsAvailable(ExpectedUrl, Arg.Any()); + + // probing first means a gone item never pays for the plex metadata round-trip + await _plexServerApiClient.DidNotReceive().GetEpisodeMetadataAndStatistics( + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any()); + } + + [Test] + public async Task Should_Return_Playout_Item_With_Url_When_Remote_Stream_Is_Available() + { + _remoteStreamProber.IsAvailable(Arg.Any(), Arg.Any()).Returns(true); + _plexServerApiClient.GetEpisodeMetadataAndStatistics( + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any()) + .Returns( + Right>( + Tuple(new EpisodeMetadata(), new MediaVersion { Name = "Main", Streams = [] }))); + + Either result = + await CreateProvider().CheckForExternalJson(_channel, _now, "/ffprobe", CancellationToken.None); + + // fail-open: an available remote stream still yields the playable /media/plex URL + result.IsRight.ShouldBeTrue(); + result.RightToSeq().Head.Path.ShouldBe(ExpectedUrl); + await _remoteStreamProber.Received(1).IsAvailable(ExpectedUrl, Arg.Any()); + } + + private ExternalJsonPlayoutItemProvider CreateProvider() => + new( + _db.Factory, + _fileSystem, + _plexPathReplacementService, + _plexServerApiClient, + _plexSecretStore, + _localStatisticsProvider, + _remoteStreamProber, + Microsoft.Extensions.Logging.Abstractions.NullLogger.Instance); + + private static string ScheduleJson() => + JsonConvert.SerializeObject( + new ExternalJsonChannel + { + // one program, a 1h window starting 1 minute before "now", so it is the current item + StartTime = "2026-01-01T11:59:00Z", + Programs = + [ + new ExternalJsonProgram + { + Type = "episode", + Duration = 3_600_000, + PlexFile = PlexFile, + File = "/plexserver/shows/example/s01e01.mkv", + ServerKey = ServerKey, + RatingKey = "12345", + Title = "Example", + ShowTitle = "Example Show", + Season = 1, + Episode = 1 + } + ] + }); + + private async Task SeedAsync() + { + await using TvContext context = _db.CreateContext(); + + var source = new PlexMediaSource + { + ServerName = ServerKey, + ClientIdentifier = ClientIdentifier, + ProductVersion = "1", + Platform = "Linux", + PlatformVersion = "1", + PathReplacements = [], + Connections = [new PlexConnection { IsActive = true, Uri = "http://plex:32400" }], + Libraries = + [ + new PlexLibrary + { + Name = "Plex Movies", + MediaKind = LibraryMediaKind.Movies, + Key = "1", + ShouldSyncItems = true, + Paths = [new LibraryPath { Path = "/plex" }] + } + ] + }; + await context.PlexMediaSources.AddAsync(source); + await context.SaveChangesAsync(); + _serverId = source.Id; + + _channel = new DomainChannel(Guid.NewGuid()) + { + Number = "1", + Name = "External JSON", + Group = "ErsatzTV", + Categories = string.Empty, + StreamSelector = string.Empty, + PreferredAudioLanguageCode = string.Empty, + PreferredAudioTitle = string.Empty, + PreferredSubtitleLanguageCode = string.Empty, + MusicVideoCreditsTemplate = string.Empty, + StreamingMode = StreamingMode.TransportStream, + PlayoutSource = ChannelPlayoutSource.Generated, + PlayoutMode = ChannelPlayoutMode.Continuous + }; + context.Channels.Add(_channel); + await context.SaveChangesAsync(); + + context.Playouts.Add( + new Playout + { + ChannelId = _channel.Id, + ScheduleKind = PlayoutScheduleKind.ExternalJson, + ScheduleFile = ScheduleFile + }); + await context.SaveChangesAsync(); + } +} diff --git a/docs/decisions.md b/docs/decisions.md index e4e4443af..0d5104a6f 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -2186,3 +2186,38 @@ protection by accident of control flow, not by design. (`MediaServer{Television,Movie,OtherVideo}LibraryScannerTests`) prove the wiring: empty incoming + non-empty existing flags nothing and reindexes nothing. Proven non-vacuous by neutralizing the guard and watching all four anti-nuke assertions fail while the `(0,0)` no-op case stays green. + +## 2026-07-20 — External-JSON playout channels now probe the remote-stream URL too, closing the #473 scope gap (#480) + +The #473 fix (PR #479, the entry above dated 2026-07-19) probed Plex/Jellyfin/Emby remote-stream URLs in +`GetPlayoutItemProcessByChannelNumberHandler.ValidatePlayoutItemPath`, but explicitly scoped itself to the +**generated-playout** path and flagged `ExternalJsonPlayoutItemProvider` as the surviving hole (tracked as +#480). That provider builds its own `/media/plex/{server}/{plexFile}` URL in `StreamRemotely` and its result +is assigned in the handler *without* passing through `ValidatePlayoutItemPath`, so for external-JSON channels +a media item gone from the server still 404'd under ffmpeg (exit 8) and the same dead item was re-selected +for its whole slot. This closes that gap. + +- **Fix location: the provider, not a new handler choke point.** `StreamRemotely` now probes the URL via the + same `IRemoteStreamProber` seam before returning it, mirroring the three generated-playout branches. The + handler-side single-choke-point option (route *every* `PlayoutItemWithPath` through one validator) was + rejected: the external-JSON provider constructs a **synthetic** `MediaItem`/`PlayoutItem` (from the JSON + + Plex API) whose path is already a `/media/plex/...` URL, so it does not fit `ValidatePlayoutItemPath`'s + local-path-then-per-provider-switch derivation without contortion. Per-provider probing keeps the blast + radius to one method; the fail-open **policy** (only a *redirected* 404 fails closed) lives entirely inside + `IRemoteStreamProber`, so the second call site duplicates only the *decision to probe*, not the policy. +- **Probe before the Plex metadata round-trip.** The URL depends only on `server.Id` + `program.PlexFile`, + not on the constructed `MediaItem`, so the probe runs *before* `GetPlexEpisode`/`GetPlexMovie`. A gone item + therefore also skips the Plex API metadata call (which would otherwise `throw NotSupportedException` on a + `Left`). +- **Error type + card sizing.** An unavailable stream returns `PlayoutItemNotAvailableFromMediaServer`, which + the handler already maps (case added in #473) to a real-error card rather than "Channel is Offline". One + behavioural difference from the generated path is accepted: external-JSON channels have **no DB + `PlayoutItem` rows** (their schedule is a JSON file), so the handler's `maybeNextStart` query returns none + and the error card falls to the existing 1-minute work-ahead clamp instead of spanning to the next item. + The worker then re-tunes and advances by time. That is repeated 1-minute error cards within a long dead + slot rather than one long card — strictly better than the exit-8 loop, and sizing to the next JSON program + would require parsing the schedule here (deferred, out of scope). +- **Tests.** `ExternalJsonPlayoutItemProviderTests` pins both directions (unavailable → `Left` + `PlayoutItemNotAvailableFromMediaServer` and no metadata call; available → `Right` with the `/media/plex` + URL). The fail-open contract itself stays pinned by `HttpRemoteStreamProberTests`. Proven non-vacuous by + neutralizing the probe guard and watching the unavailable assertion flip to `Right`.