From a1bd303cce2c6e101553aa2791a56e71a1d05cb3 Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 11 Jul 2026 17:42:45 +0200 Subject: [PATCH 01/12] fix(app): post-commit side effects on CancellationToken.None + guide-xml/empty-list hardening (#254) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backend mutation-hardening cluster from the 2026-07-11 mutation-safety audit sweep (adversarial-reviewer #22/#23), the parallel-safe backend-isolated slice. audit#22 F4 — standardize post-commit enqueues on CancellationToken.None: 20 command handlers under MediaCollections/, ProgramSchedules/, Playouts/, Channels/ threaded the request cancellationToken into work that runs AFTER SaveChangesAsync commits (WriteAsync rebuild/refresh enqueues, mediator.Publish, reindex, cache Refresh, and post-commit lookups that gate an enqueue). A late client-disconnect then turns an already-durable commit into a thrown request AND drops the side effect. Generalizes the #251 deco-handler fix. Excludes BuildPlayoutHandler (worker/background token, not a client-disconnect token), the config/FFmpeg multi-upsert handlers (partial-commit case, separate follow-up), and response-projection reloads (correctly keep the request token). audit#22 F2 — DeleteChannelHandler/DeletePlayoutHandler now delete the channel guide {number}.xml through IFileSystem.File.Delete (observable under MockFileSystem) and BEFORE the commit (a post-commit delete orphans the xml on a crash; the xml is regenerable on demand, so pre-commit delete is the safe order). audit#23 F4 — ReplacePlayoutAlternateScheduleItemsHandler rejects an empty item list in the handler (not only the controller pre-guard) so a direct caller can't trip the Max()-on-empty crash. Docs: api-conventions.md §7a (post-commit token convention + boundaries), decisions.md entry (rationale, sweep scope, #253 PR2-4 coordination note). Tests: guide-cache-delete-through-FS for both delete handlers, empty-list guard. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Commands/BulkDeleteChannelsHandler.cs | 4 +- .../BulkMoveChannelsToGroupHandler.cs | 5 +- .../CreateChannelFromLineupHandler.cs | 9 ++-- .../Channels/Commands/DeleteChannelHandler.cs | 22 +++++---- .../Channels/Commands/UpdateChannelHandler.cs | 10 ++-- .../Commands/UpdateChannelNumbersHandler.cs | 6 ++- .../Commands/AddItemsToCollectionHandler.cs | 6 ++- .../RemoveItemsFromCollectionHandler.cs | 6 ++- .../Commands/UpdateCollectionHandler.cs | 4 +- .../Commands/UpdateMultiCollectionHandler.cs | 4 +- .../Commands/UpdateRerunCollectionHandler.cs | 4 +- .../Commands/UpdateSmartCollectionHandler.cs | 7 ++- .../Commands/UpdateTraktListHandler.cs | 4 +- .../Commands/CreateScriptedPlayoutHandler.cs | 9 ++-- .../CreateSequentialPlayoutHandler.cs | 9 ++-- .../Playouts/Commands/DeletePlayoutHandler.cs | 21 +++++---- ...acePlayoutAlternateScheduleItemsHandler.cs | 12 ++++- .../UpdateExternalJsonPlayoutHandler.cs | 4 +- .../Commands/UpdateScriptedPlayoutHandler.cs | 4 +- .../UpdateSequentialPlayoutHandler.cs | 4 +- .../Commands/AddProgramScheduleItemHandler.cs | 4 +- .../DeleteProgramScheduleItemHandler.cs | 4 +- .../ReplaceProgramScheduleItemsHandler.cs | 4 +- .../Channels/DeleteChannelHandlerTests.cs | 22 ++++++++- .../Playouts/PlayoutHandlerTests.cs | 47 +++++++++++++++++++ docs/api-conventions.md | 32 +++++++++++++ docs/decisions.md | 41 ++++++++++++++++ 27 files changed, 255 insertions(+), 53 deletions(-) diff --git a/ErsatzTV.Application/Channels/Commands/BulkDeleteChannelsHandler.cs b/ErsatzTV.Application/Channels/Commands/BulkDeleteChannelsHandler.cs index 58dff8705..9cd4369eb 100644 --- a/ErsatzTV.Application/Channels/Commands/BulkDeleteChannelsHandler.cs +++ b/ErsatzTV.Application/Channels/Commands/BulkDeleteChannelsHandler.cs @@ -55,7 +55,9 @@ public class BulkDeleteChannelsHandler( } } - await workerChannel.WriteAsync(new RefreshChannelList(), cancellationToken); + // post-commit side effect runs on CancellationToken.None so a late request cancellation + // can't abort it after the commit landed (#254) + await workerChannel.WriteAsync(new RefreshChannelList(), CancellationToken.None); return Right(Unit.Default); } diff --git a/ErsatzTV.Application/Channels/Commands/BulkMoveChannelsToGroupHandler.cs b/ErsatzTV.Application/Channels/Commands/BulkMoveChannelsToGroupHandler.cs index ede6c4506..1e78ae609 100644 --- a/ErsatzTV.Application/Channels/Commands/BulkMoveChannelsToGroupHandler.cs +++ b/ErsatzTV.Application/Channels/Commands/BulkMoveChannelsToGroupHandler.cs @@ -54,7 +54,10 @@ public class BulkMoveChannelsToGroupHandler( await transaction.CommitAsync(cancellationToken); searchTargets.SearchTargetsChanged(); - await workerChannel.WriteAsync(new RefreshChannelList(), cancellationToken); + + // post-commit side effect runs on CancellationToken.None so a late request cancellation + // can't abort it after the commit landed (#254) + await workerChannel.WriteAsync(new RefreshChannelList(), CancellationToken.None); return Right(Unit.Default); } diff --git a/ErsatzTV.Application/Channels/Commands/CreateChannelFromLineupHandler.cs b/ErsatzTV.Application/Channels/Commands/CreateChannelFromLineupHandler.cs index fd8773482..4127dc10f 100644 --- a/ErsatzTV.Application/Channels/Commands/CreateChannelFromLineupHandler.cs +++ b/ErsatzTV.Application/Channels/Commands/CreateChannelFromLineupHandler.cs @@ -68,19 +68,22 @@ public class CreateChannelFromLineupHandler( } searchTargets.SearchTargetsChanged(); + + // post-commit side effect runs on CancellationToken.None so a late request cancellation + // can't abort it after the commit landed (#254) await workerChannel.WriteAsync( new BuildPlayout(prepared.Playout.Id, PlayoutBuildMode.Reset), - cancellationToken); + CancellationToken.None); // Mirror CreateClassicPlayoutHandler: on-demand playouts must be time-shifted to "now" after build. if (prepared.Channel.PlayoutMode is ChannelPlayoutMode.OnDemand) { await workerChannel.WriteAsync( new TimeShiftOnDemandPlayout(prepared.Playout.Id, DateTimeOffset.Now, false), - cancellationToken); + CancellationToken.None); } - await workerChannel.WriteAsync(new RefreshChannelList(), cancellationToken); + await workerChannel.WriteAsync(new RefreshChannelList(), CancellationToken.None); return new CreateChannelFromLineupResponseModel( prepared.Channel.Id, diff --git a/ErsatzTV.Application/Channels/Commands/DeleteChannelHandler.cs b/ErsatzTV.Application/Channels/Commands/DeleteChannelHandler.cs index 0363cecab..bd673ef22 100644 --- a/ErsatzTV.Application/Channels/Commands/DeleteChannelHandler.cs +++ b/ErsatzTV.Application/Channels/Commands/DeleteChannelHandler.cs @@ -47,20 +47,24 @@ public class DeleteChannelHandler : IRequestHandler DoDeletion(TvContext dbContext, Channel channel, CancellationToken cancellationToken) { + // Delete the guide cache file through the filesystem abstraction (so it's observable under a + // MockFileSystem) and BEFORE the commit: deleting after commit orphans {number}.xml if the + // process crashes in between (nothing reaps it, and GetChannelGuideHandler serves everything + // in the cache folder). The guide xml is regenerable on demand, so losing it pre-commit is safe (#254). + string cacheFile = Path.Combine(FileSystemLayout.ChannelGuideCacheFolder, $"{channel.Number}.xml"); + if (_fileSystem.File.Exists(cacheFile)) + { + _fileSystem.File.Delete(cacheFile); + } + dbContext.Channels.Remove(channel); await dbContext.SaveChangesAsync(cancellationToken); _searchTargets.SearchTargetsChanged(); - // delete channel data from channel guide cache - string cacheFile = Path.Combine(FileSystemLayout.ChannelGuideCacheFolder, $"{channel.Number}.xml"); - if (_fileSystem.File.Exists(cacheFile)) - { - File.Delete(cacheFile); - } - - // refresh channel list to remove channel that has no playout - await _workerChannel.WriteAsync(new RefreshChannelList(), cancellationToken); + // refresh channel list to remove channel that has no playout — post-commit side effect runs on + // CancellationToken.None so a late request cancellation can't abort it after the delete committed (#254) + await _workerChannel.WriteAsync(new RefreshChannelList(), CancellationToken.None); return Unit.Default; } diff --git a/ErsatzTV.Application/Channels/Commands/UpdateChannelHandler.cs b/ErsatzTV.Application/Channels/Commands/UpdateChannelHandler.cs index bf3039a4e..39dfc1ad8 100644 --- a/ErsatzTV.Application/Channels/Commands/UpdateChannelHandler.cs +++ b/ErsatzTV.Application/Channels/Commands/UpdateChannelHandler.cs @@ -157,21 +157,23 @@ public class UpdateChannelHandler( searchTargets.SearchTargetsChanged(); + // post-commit side effect runs on CancellationToken.None so a late request cancellation + // can't abort it after the commit landed (#254) if (c.SubtitleMode != ChannelSubtitleMode.None) { Option maybePlayout = await dbContext.Playouts - .SelectOneAsync(p => p.ChannelId, p => p.ChannelId == c.Id, cancellationToken); + .SelectOneAsync(p => p.ChannelId, p => p.ChannelId == c.Id, CancellationToken.None); foreach (Playout playout in maybePlayout) { - await workerChannel.WriteAsync(new ExtractEmbeddedSubtitles(playout.Id), cancellationToken); + await workerChannel.WriteAsync(new ExtractEmbeddedSubtitles(playout.Id), CancellationToken.None); } } - await workerChannel.WriteAsync(new RefreshChannelList(), cancellationToken); + await workerChannel.WriteAsync(new RefreshChannelList(), CancellationToken.None); if (hasEpgChange) { - await workerChannel.WriteAsync(new RefreshChannelData(c.Number), cancellationToken); + await workerChannel.WriteAsync(new RefreshChannelData(c.Number), CancellationToken.None); } return ProjectToViewModel(c, c.Playouts?.Count ?? 0); diff --git a/ErsatzTV.Application/Channels/Commands/UpdateChannelNumbersHandler.cs b/ErsatzTV.Application/Channels/Commands/UpdateChannelNumbersHandler.cs index c1873c15f..3fdc321b8 100644 --- a/ErsatzTV.Application/Channels/Commands/UpdateChannelNumbersHandler.cs +++ b/ErsatzTV.Application/Channels/Commands/UpdateChannelNumbersHandler.cs @@ -81,10 +81,12 @@ public class UpdateChannelNumbersHandler( await transaction.CommitAsync(cancellationToken); // update channel list and xmltv - await workerChannel.WriteAsync(new RefreshChannelList(), cancellationToken); + // post-commit side effect runs on CancellationToken.None so a late request cancellation + // can't abort it after the commit landed (#254) + await workerChannel.WriteAsync(new RefreshChannelList(), CancellationToken.None); foreach (var channel in channelsToUpdate) { - await workerChannel.WriteAsync(new RefreshChannelData(channel.Number), cancellationToken); + await workerChannel.WriteAsync(new RefreshChannelData(channel.Number), CancellationToken.None); } return Option.None; diff --git a/ErsatzTV.Application/MediaCollections/Commands/AddItemsToCollectionHandler.cs b/ErsatzTV.Application/MediaCollections/Commands/AddItemsToCollectionHandler.cs index b21ce9f6f..475a1494d 100644 --- a/ErsatzTV.Application/MediaCollections/Commands/AddItemsToCollectionHandler.cs +++ b/ErsatzTV.Application/MediaCollections/Commands/AddItemsToCollectionHandler.cs @@ -81,13 +81,15 @@ public class AddItemsToCollectionHandler : if (await dbContext.SaveChangesAsync(cancellationToken) > 0) { - await _searchChannel.WriteAsync(new ReindexMediaItems(toAddIds.ToArray()), cancellationToken); + // post-commit side effect runs on CancellationToken.None so a late request cancellation + // can't abort it after the commit landed (#254) + await _searchChannel.WriteAsync(new ReindexMediaItems(toAddIds.ToArray()), CancellationToken.None); // refresh all playouts that use this collection foreach (int playoutId in await _mediaCollectionRepository .PlayoutIdsUsingCollection(request.CollectionId)) { - await _channel.WriteAsync(new BuildPlayout(playoutId, PlayoutBuildMode.Refresh), cancellationToken); + await _channel.WriteAsync(new BuildPlayout(playoutId, PlayoutBuildMode.Refresh), CancellationToken.None); } } diff --git a/ErsatzTV.Application/MediaCollections/Commands/RemoveItemsFromCollectionHandler.cs b/ErsatzTV.Application/MediaCollections/Commands/RemoveItemsFromCollectionHandler.cs index 58e267ae6..24af4aaae 100644 --- a/ErsatzTV.Application/MediaCollections/Commands/RemoveItemsFromCollectionHandler.cs +++ b/ErsatzTV.Application/MediaCollections/Commands/RemoveItemsFromCollectionHandler.cs @@ -63,14 +63,16 @@ public class RemoveItemsFromCollectionHandler : IRequestHandler 0) { + // post-commit side effect runs on CancellationToken.None so a late request cancellation + // can't abort it after the commit landed (#254) await _searchChannel.WriteAsync( new ReindexMediaItems(itemsToRemove.Select(mi => mi.Id).ToArray()), - cancellationToken); + CancellationToken.None); // refresh all playouts that use this collection foreach (int playoutId in await _mediaCollectionRepository.PlayoutIdsUsingCollection(collection.Id)) { - await _channel.WriteAsync(new BuildPlayout(playoutId, PlayoutBuildMode.Refresh), cancellationToken); + await _channel.WriteAsync(new BuildPlayout(playoutId, PlayoutBuildMode.Refresh), CancellationToken.None); } } diff --git a/ErsatzTV.Application/MediaCollections/Commands/UpdateCollectionHandler.cs b/ErsatzTV.Application/MediaCollections/Commands/UpdateCollectionHandler.cs index 15010dbe6..73ca500dd 100644 --- a/ErsatzTV.Application/MediaCollections/Commands/UpdateCollectionHandler.cs +++ b/ErsatzTV.Application/MediaCollections/Commands/UpdateCollectionHandler.cs @@ -61,11 +61,13 @@ public class UpdateCollectionHandler : IRequestHandler 0 && request.UseCustomPlaybackOrder.IsSome) { + // post-commit side effect runs on CancellationToken.None so a late request cancellation + // can't abort it after the commit landed (#254) // refresh all playouts that use this collection foreach (int playoutId in await _mediaCollectionRepository.PlayoutIdsUsingCollection( request.CollectionId)) { - await _channel.WriteAsync(new BuildPlayout(playoutId, PlayoutBuildMode.Refresh), cancellationToken); + await _channel.WriteAsync(new BuildPlayout(playoutId, PlayoutBuildMode.Refresh), CancellationToken.None); } } diff --git a/ErsatzTV.Application/MediaCollections/Commands/UpdateMultiCollectionHandler.cs b/ErsatzTV.Application/MediaCollections/Commands/UpdateMultiCollectionHandler.cs index d29987398..f4af1f30a 100644 --- a/ErsatzTV.Application/MediaCollections/Commands/UpdateMultiCollectionHandler.cs +++ b/ErsatzTV.Application/MediaCollections/Commands/UpdateMultiCollectionHandler.cs @@ -124,10 +124,12 @@ public class UpdateMultiCollectionHandler : IRequestHandler 0) { + // post-commit side effect runs on CancellationToken.None so a late request cancellation + // can't abort it after the commit landed (#254) // refresh all playouts that use this rerun collection foreach (int playoutId in await mediaCollectionRepository.PlayoutIdsUsingRerunCollection( request.RerunCollectionId)) { - await channel.WriteAsync(new BuildPlayout(playoutId, PlayoutBuildMode.Refresh), cancellationToken); + await channel.WriteAsync(new BuildPlayout(playoutId, PlayoutBuildMode.Refresh), CancellationToken.None); } } diff --git a/ErsatzTV.Application/MediaCollections/Commands/UpdateSmartCollectionHandler.cs b/ErsatzTV.Application/MediaCollections/Commands/UpdateSmartCollectionHandler.cs index e1cfbdc9d..df7e06246 100644 --- a/ErsatzTV.Application/MediaCollections/Commands/UpdateSmartCollectionHandler.cs +++ b/ErsatzTV.Application/MediaCollections/Commands/UpdateSmartCollectionHandler.cs @@ -69,12 +69,15 @@ public class if (await dbContext.SaveChangesAsync(cancellationToken) > 0) { _searchTargets.SearchTargetsChanged(); - await _smartCollectionCache.Refresh(cancellationToken); + + // post-commit side effects run on CancellationToken.None so a late request cancellation + // can't abort them after the commit landed (#254) + await _smartCollectionCache.Refresh(CancellationToken.None); // refresh all playouts that use this smart collection foreach (int playoutId in await _mediaCollectionRepository.PlayoutIdsUsingSmartCollection(request.Id)) { - await _channel.WriteAsync(new BuildPlayout(playoutId, PlayoutBuildMode.Refresh), cancellationToken); + await _channel.WriteAsync(new BuildPlayout(playoutId, PlayoutBuildMode.Refresh), CancellationToken.None); } } diff --git a/ErsatzTV.Application/MediaCollections/Commands/UpdateTraktListHandler.cs b/ErsatzTV.Application/MediaCollections/Commands/UpdateTraktListHandler.cs index 772d5ed62..f24e2805a 100644 --- a/ErsatzTV.Application/MediaCollections/Commands/UpdateTraktListHandler.cs +++ b/ErsatzTV.Application/MediaCollections/Commands/UpdateTraktListHandler.cs @@ -57,7 +57,9 @@ public class UpdateTraktListHandler( { try { - await workerChannel.WriteAsync(new MatchTraktListItems(traktList.Id), cancellationToken); + // post-commit side effect runs on CancellationToken.None so a late request + // cancellation can't abort it after the commit landed (#254) + await workerChannel.WriteAsync(new MatchTraktListItems(traktList.Id), CancellationToken.None); } catch { diff --git a/ErsatzTV.Application/Playouts/Commands/CreateScriptedPlayoutHandler.cs b/ErsatzTV.Application/Playouts/Commands/CreateScriptedPlayoutHandler.cs index 88dc41e20..8af0f2606 100644 --- a/ErsatzTV.Application/Playouts/Commands/CreateScriptedPlayoutHandler.cs +++ b/ErsatzTV.Application/Playouts/Commands/CreateScriptedPlayoutHandler.cs @@ -35,15 +35,18 @@ public class CreateScriptedPlayoutHandler( { await dbContext.Playouts.AddAsync(playout, cancellationToken); await dbContext.SaveChangesAsync(cancellationToken); - await channel.WriteAsync(new BuildPlayout(playout.Id, PlayoutBuildMode.Reset), cancellationToken); + + // post-commit side effect runs on CancellationToken.None so a late request cancellation + // can't abort it after the commit landed (#254) + await channel.WriteAsync(new BuildPlayout(playout.Id, PlayoutBuildMode.Reset), CancellationToken.None); if (playout.Channel.PlayoutMode is ChannelPlayoutMode.OnDemand) { await channel.WriteAsync( new TimeShiftOnDemandPlayout(playout.Id, DateTimeOffset.Now, false), - cancellationToken); + CancellationToken.None); } - await channel.WriteAsync(new RefreshChannelList(), cancellationToken); + await channel.WriteAsync(new RefreshChannelList(), CancellationToken.None); return new CreatePlayoutResponse(playout.Id); } diff --git a/ErsatzTV.Application/Playouts/Commands/CreateSequentialPlayoutHandler.cs b/ErsatzTV.Application/Playouts/Commands/CreateSequentialPlayoutHandler.cs index 267a22098..c0ce15b2e 100644 --- a/ErsatzTV.Application/Playouts/Commands/CreateSequentialPlayoutHandler.cs +++ b/ErsatzTV.Application/Playouts/Commands/CreateSequentialPlayoutHandler.cs @@ -33,15 +33,18 @@ public class CreateSequentialPlayoutHandler( { await dbContext.Playouts.AddAsync(playout, cancellationToken); await dbContext.SaveChangesAsync(cancellationToken); - await channel.WriteAsync(new BuildPlayout(playout.Id, PlayoutBuildMode.Reset), cancellationToken); + + // post-commit side effect runs on CancellationToken.None so a late request cancellation + // can't abort it after the commit landed (#254) + await channel.WriteAsync(new BuildPlayout(playout.Id, PlayoutBuildMode.Reset), CancellationToken.None); if (playout.Channel.PlayoutMode is ChannelPlayoutMode.OnDemand) { await channel.WriteAsync( new TimeShiftOnDemandPlayout(playout.Id, DateTimeOffset.Now, false), - cancellationToken); + CancellationToken.None); } - await channel.WriteAsync(new RefreshChannelList(), cancellationToken); + await channel.WriteAsync(new RefreshChannelList(), CancellationToken.None); return new CreatePlayoutResponse(playout.Id); } diff --git a/ErsatzTV.Application/Playouts/Commands/DeletePlayoutHandler.cs b/ErsatzTV.Application/Playouts/Commands/DeletePlayoutHandler.cs index c6434eaff..4831de8ce 100644 --- a/ErsatzTV.Application/Playouts/Commands/DeletePlayoutHandler.cs +++ b/ErsatzTV.Application/Playouts/Commands/DeletePlayoutHandler.cs @@ -28,20 +28,25 @@ public class DeletePlayoutHandler( foreach (Playout playout in maybePlayout) { - dbContext.Playouts.Remove(playout); - await dbContext.SaveChangesAsync(cancellationToken); - - // delete channel data from channel guide cache + // Delete the guide cache file through the filesystem abstraction (observable under a + // MockFileSystem) and BEFORE the commit: deleting after commit orphans {number}.xml if the + // process crashes in between. The guide xml is regenerable on demand, so losing it pre-commit + // is safe (#254). string cacheFile = Path.Combine(FileSystemLayout.ChannelGuideCacheFolder, $"{playout.Channel.Number}.xml"); if (fileSystem.File.Exists(cacheFile)) { - File.Delete(cacheFile); + fileSystem.File.Delete(cacheFile); } - // refresh channel list to remove channel that has no playout - await workerChannel.WriteAsync(new RefreshChannelList(), cancellationToken); + dbContext.Playouts.Remove(playout); + await dbContext.SaveChangesAsync(cancellationToken); - await mediator.Publish(new PlayoutUpdatedNotification(playout.Id, false), cancellationToken); + // post-commit side effects run on CancellationToken.None so a late request cancellation can't + // abort them after the delete committed (#254) + // refresh channel list to remove channel that has no playout + await workerChannel.WriteAsync(new RefreshChannelList(), CancellationToken.None); + + await mediator.Publish(new PlayoutUpdatedNotification(playout.Id, false), CancellationToken.None); } return maybePlayout diff --git a/ErsatzTV.Application/Playouts/Commands/ReplacePlayoutAlternateScheduleItemsHandler.cs b/ErsatzTV.Application/Playouts/Commands/ReplacePlayoutAlternateScheduleItemsHandler.cs index 0cf6490a6..434a0e439 100644 --- a/ErsatzTV.Application/Playouts/Commands/ReplacePlayoutAlternateScheduleItemsHandler.cs +++ b/ErsatzTV.Application/Playouts/Commands/ReplacePlayoutAlternateScheduleItemsHandler.cs @@ -20,7 +20,13 @@ public class ReplacePlayoutAlternateScheduleItemsHandler( ReplacePlayoutAlternateScheduleItems request, CancellationToken cancellationToken) { - // TODO: validate that items is not empty + // The handler reads the highest-index item as the default schedule (Max() below), so an empty + // list is invalid — reject it here rather than letting Max() throw. The controller pre-guards + // too, but a direct handler caller (MCP, test, reuse) must get a clean 422, not a raw crash (#254). + if (request.Items.Count == 0) + { + return BaseError.New("Playout alternate schedule items must not be empty"); + } try { @@ -152,9 +158,11 @@ public class ReplacePlayoutAlternateScheduleItemsHandler( existingValue.Name, schedule.Name); + // post-commit enqueue runs on CancellationToken.None: the schedule change is + // already committed, so a late cancellation must not drop the rebuild (#254) await channel.WriteAsync( new BuildPlayout(request.PlayoutId, PlayoutBuildMode.Refresh), - cancellationToken); + CancellationToken.None); break; } diff --git a/ErsatzTV.Application/Playouts/Commands/UpdateExternalJsonPlayoutHandler.cs b/ErsatzTV.Application/Playouts/Commands/UpdateExternalJsonPlayoutHandler.cs index 81eea96fd..c94770f57 100644 --- a/ErsatzTV.Application/Playouts/Commands/UpdateExternalJsonPlayoutHandler.cs +++ b/ErsatzTV.Application/Playouts/Commands/UpdateExternalJsonPlayoutHandler.cs @@ -42,7 +42,9 @@ public class if (await dbContext.SaveChangesAsync(cancellationToken) > 0) { - await _workerChannel.WriteAsync(new RefreshChannelData(playout.Channel.Number), cancellationToken); + // post-commit side effect runs on CancellationToken.None so a late request cancellation + // can't abort it after the commit landed (#254) + await _workerChannel.WriteAsync(new RefreshChannelData(playout.Channel.Number), CancellationToken.None); } return new PlayoutNameViewModel( diff --git a/ErsatzTV.Application/Playouts/Commands/UpdateScriptedPlayoutHandler.cs b/ErsatzTV.Application/Playouts/Commands/UpdateScriptedPlayoutHandler.cs index 329fc33b3..9ef7c1c58 100644 --- a/ErsatzTV.Application/Playouts/Commands/UpdateScriptedPlayoutHandler.cs +++ b/ErsatzTV.Application/Playouts/Commands/UpdateScriptedPlayoutHandler.cs @@ -37,7 +37,9 @@ public class if (await dbContext.SaveChangesAsync(cancellationToken) > 0) { - await workerChannel.WriteAsync(new RefreshChannelData(playout.Channel.Number), cancellationToken); + // post-commit side effect runs on CancellationToken.None so a late request cancellation + // can't abort it after the commit landed (#254) + await workerChannel.WriteAsync(new RefreshChannelData(playout.Channel.Number), CancellationToken.None); } return new PlayoutNameViewModel( diff --git a/ErsatzTV.Application/Playouts/Commands/UpdateSequentialPlayoutHandler.cs b/ErsatzTV.Application/Playouts/Commands/UpdateSequentialPlayoutHandler.cs index 1f50282ab..e81cb6ab9 100644 --- a/ErsatzTV.Application/Playouts/Commands/UpdateSequentialPlayoutHandler.cs +++ b/ErsatzTV.Application/Playouts/Commands/UpdateSequentialPlayoutHandler.cs @@ -42,7 +42,9 @@ public class if (await dbContext.SaveChangesAsync(cancellationToken) > 0) { - await _workerChannel.WriteAsync(new RefreshChannelData(playout.Channel.Number), cancellationToken); + // post-commit side effect runs on CancellationToken.None so a late request cancellation + // can't abort it after the commit landed (#254) + await _workerChannel.WriteAsync(new RefreshChannelData(playout.Channel.Number), CancellationToken.None); } return new PlayoutNameViewModel( diff --git a/ErsatzTV.Application/ProgramSchedules/Commands/AddProgramScheduleItemHandler.cs b/ErsatzTV.Application/ProgramSchedules/Commands/AddProgramScheduleItemHandler.cs index 1ec7ceb71..b8656406a 100644 --- a/ErsatzTV.Application/ProgramSchedules/Commands/AddProgramScheduleItemHandler.cs +++ b/ErsatzTV.Application/ProgramSchedules/Commands/AddProgramScheduleItemHandler.cs @@ -55,9 +55,11 @@ public class AddProgramScheduleItemHandler : ProgramScheduleItemCommandBase, await dbContext.SaveChangesAsync(cancellationToken); // refresh any playouts that use this schedule + // post-commit side effect runs on CancellationToken.None so a late request cancellation + // can't abort it after the commit landed (#254) foreach (Playout playout in programSchedule.Playouts) { - await _channel.WriteAsync(new BuildPlayout(playout.Id, PlayoutBuildMode.Refresh), cancellationToken); + await _channel.WriteAsync(new BuildPlayout(playout.Id, PlayoutBuildMode.Refresh), CancellationToken.None); } // reload with the full navigation graph before projecting: BuildItem creates the watermark/graphics diff --git a/ErsatzTV.Application/ProgramSchedules/Commands/DeleteProgramScheduleItemHandler.cs b/ErsatzTV.Application/ProgramSchedules/Commands/DeleteProgramScheduleItemHandler.cs index f54ccdd5f..017f64ac5 100644 --- a/ErsatzTV.Application/ProgramSchedules/Commands/DeleteProgramScheduleItemHandler.cs +++ b/ErsatzTV.Application/ProgramSchedules/Commands/DeleteProgramScheduleItemHandler.cs @@ -36,9 +36,11 @@ public class DeleteProgramScheduleItemHandler( dbContext.ProgramScheduleItems.Remove(item); await dbContext.SaveChangesAsync(cancellationToken); + // post-commit side effect runs on CancellationToken.None so a late request cancellation + // can't abort it after the commit landed (#254) foreach (Playout playout in playouts) { - await channel.WriteAsync(new BuildPlayout(playout.Id, PlayoutBuildMode.Refresh), cancellationToken); + await channel.WriteAsync(new BuildPlayout(playout.Id, PlayoutBuildMode.Refresh), CancellationToken.None); } return Unit.Default; diff --git a/ErsatzTV.Application/ProgramSchedules/Commands/ReplaceProgramScheduleItemsHandler.cs b/ErsatzTV.Application/ProgramSchedules/Commands/ReplaceProgramScheduleItemsHandler.cs index 0ffc63e05..f68ac5e73 100644 --- a/ErsatzTV.Application/ProgramSchedules/Commands/ReplaceProgramScheduleItemsHandler.cs +++ b/ErsatzTV.Application/ProgramSchedules/Commands/ReplaceProgramScheduleItemsHandler.cs @@ -95,9 +95,11 @@ public class ReplaceProgramScheduleItemsHandler( await dbContext.SaveChangesAsync(cancellationToken); // refresh any playouts that use this schedule + // post-commit side effect runs on CancellationToken.None so a late request cancellation + // can't abort it after the commit landed (#254) foreach (Playout playout in programSchedule.Playouts) { - await channel.WriteAsync(new BuildPlayout(playout.Id, PlayoutBuildMode.Refresh), cancellationToken); + await channel.WriteAsync(new BuildPlayout(playout.Id, PlayoutBuildMode.Refresh), CancellationToken.None); } // reload with the full navigation graph before projecting: BuildItem creates the watermark/graphics diff --git a/ErsatzTV.Tests/Application/Channels/DeleteChannelHandlerTests.cs b/ErsatzTV.Tests/Application/Channels/DeleteChannelHandlerTests.cs index 3fbb057dc..9b7142af0 100644 --- a/ErsatzTV.Tests/Application/Channels/DeleteChannelHandlerTests.cs +++ b/ErsatzTV.Tests/Application/Channels/DeleteChannelHandlerTests.cs @@ -14,7 +14,8 @@ namespace ErsatzTV.Tests.Application.Channels; [TestFixture] public class DeleteChannelHandlerTests : ChannelHandlerTestBase { - private DeleteChannelHandler MakeHandler() => new(Worker, Db.Factory, new MockFileSystem(), SearchTargets); + private DeleteChannelHandler MakeHandler(MockFileSystem? fileSystem = null) => + new(Worker, Db.Factory, fileSystem ?? new MockFileSystem(), SearchTargets); [Test] public async Task Should_Return_NotFoundError_When_Channel_Missing() @@ -39,6 +40,25 @@ public class DeleteChannelHandlerTests : ChannelHandlerTestBase exists.ShouldBeFalse(); } + [Test] + public async Task Should_Delete_Guide_Cache_File_Through_FileSystem_Abstraction() + { + await SeedChannel(1, "5"); + + var fileSystem = new MockFileSystem(); + string cacheFile = fileSystem.Path.Combine(FileSystemLayout.ChannelGuideCacheFolder, "5.xml"); + fileSystem.Directory.CreateDirectory(FileSystemLayout.ChannelGuideCacheFolder); + await fileSystem.File.WriteAllTextAsync(cacheFile, ""); + fileSystem.File.Exists(cacheFile).ShouldBeTrue(); + + Either result = + await MakeHandler(fileSystem).Handle(new DeleteChannel(1), CancellationToken.None); + + result.IsRight.ShouldBeTrue(); + // routed through the abstraction (not static File.Delete), so the MockFileSystem observes the removal + fileSystem.File.Exists(cacheFile).ShouldBeFalse(); + } + private static BaseError LeftOf(Either either) => either.Match(Left: e => e, Right: _ => throw new AssertionException("Expected a Left result")); } diff --git a/ErsatzTV.Tests/Application/Playouts/PlayoutHandlerTests.cs b/ErsatzTV.Tests/Application/Playouts/PlayoutHandlerTests.cs index 5377658e5..76a6fd8cc 100644 --- a/ErsatzTV.Tests/Application/Playouts/PlayoutHandlerTests.cs +++ b/ErsatzTV.Tests/Application/Playouts/PlayoutHandlerTests.cs @@ -9,6 +9,7 @@ using ErsatzTV.Infrastructure.Data; using ErsatzTV.Tests.Support; using LanguageExt; using MediatR; +using Microsoft.Extensions.Logging.Abstractions; using NSubstitute; using NUnit.Framework; using Shouldly; @@ -71,6 +72,52 @@ public class PlayoutHandlerTests LeftOf(result).ShouldBeOfType(); } + [Test] + public async Task Delete_Should_Remove_Guide_Cache_File_Through_FileSystem_Abstraction() + { + int channelId = await SeedChannel(); + int playoutId; + await using (TvContext context = _db.CreateContext()) + { + var playout = new Playout { ChannelId = channelId, Items = [] }; + context.Playouts.Add(playout); + await context.SaveChangesAsync(); + playoutId = playout.Id; + } + + var fileSystem = new MockFileSystem(); + string cacheFile = fileSystem.Path.Combine(FileSystemLayout.ChannelGuideCacheFolder, "101.xml"); + fileSystem.Directory.CreateDirectory(FileSystemLayout.ChannelGuideCacheFolder); + await fileSystem.File.WriteAllTextAsync(cacheFile, ""); + + var handler = new DeletePlayoutHandler( + _worker, + _db.Factory, + fileSystem, + Substitute.For()); + + Either result = await handler.Handle(new DeletePlayout(playoutId), CancellationToken.None); + + result.IsRight.ShouldBeTrue(); + // routed through the abstraction (not static File.Delete), so the MockFileSystem observes the removal + fileSystem.File.Exists(cacheFile).ShouldBeFalse(); + } + + [Test] + public async Task ReplaceAlternateScheduleItems_Should_Reject_Empty_List() + { + var handler = new ReplacePlayoutAlternateScheduleItemsHandler( + _db.Factory, + _worker, + NullLogger.Instance); + + Either result = await handler.Handle( + new ReplacePlayoutAlternateScheduleItems(1, []), + CancellationToken.None); + + LeftOf(result).Value.ShouldContain("must not be empty"); + } + private static BaseError LeftOf(Either either) => either.Match(Left: e => e, Right: _ => throw new AssertionException("Expected a Left result")); diff --git a/docs/api-conventions.md b/docs/api-conventions.md index 69f99086c..33d0f7baa 100644 --- a/docs/api-conventions.md +++ b/docs/api-conventions.md @@ -258,6 +258,38 @@ via `ProgramScheduleItemQueryExtensions.IncludeScheduleItemDetails()` (the one i the NRE; the controller's `.ToList()`/serialization does (regression: `ScheduleItemWriteProjectionTests`). GET handlers that feed an ordered list must also `.OrderBy(i => i.Index)` — id order is not index order. +### 7a. Post-commit side effects run on `CancellationToken.None` + +Once a command handler's `await dbContext.SaveChangesAsync(cancellationToken)` (or repository upsert) +has **committed**, everything that runs afterwards to complete that mutation's side effect — +`channel.WriteAsync(new BuildPlayout(...))` / other worker-channel enqueues, `mediator.Publish(...)`, +`ISearchIndex`/reindex enqueues, a cache `Refresh(...)`, and any post-commit **lookup that gates one +of those enqueues** — must be passed **`CancellationToken.None`**, not the request `cancellationToken`. + +Rationale (audit #22, issues #251 → #254): the request token is cancelled when the HTTP client +disconnects. If it's threaded into a post-commit enqueue, a late disconnect turns an *already-durable* +commit into a thrown request **and drops the side effect** (e.g. the rebuild is never queued → the +persisted change silently never takes visible effect). The commit is the point of no return: past it, +the compensating side effect must not be half-abortable. Exemplar idiom: +`ErsatzTV.Application/Scheduling/Commands/UpdateDecoHandler.cs` (the affected-playout queries **and** the +`WriteAsync(BuildPlayout..., CancellationToken.None)` enqueue all use `None`). + +Two boundaries: +- **Response projection is NOT a side effect.** The post-commit reload that builds the *returned* view + model (§7 above) legitimately keeps the request `cancellationToken` — if the client disconnected, we + don't need to compute a response nobody will read, and the durable work (commit + `None`-enqueue) has + already happened. Only the *side effect* chain gets `None`. +- **Background-job handlers keep their token.** `BuildPlayoutHandler` and other handlers invoked by the + worker (not by an HTTP request) receive the *worker's* shutdown token, not a client-disconnect token — + their downstream enqueues correctly stay on that token so a shutdown stops enqueuing more work. + +Pre-commit reads/validation and the `SaveChangesAsync` call itself keep the request token (cancelling +*before* the commit safely aborts with nothing persisted). A handler that passes **no** token to a +post-commit `WriteAsync()` is already behaviorally correct (`default` == `CancellationToken.None`); +making it explicit is optional cleanup, not required. Config handlers that commit via several sequential +`IConfigElementRepository.Upsert` calls are a distinct partial-commit-under-cancellation case not covered +by this rule (tracked separately). + ## 8. Known API warts (don't "fix" without discussion — they're deliberate synthesized rows) `GET /api/blocks` and `GET /api/templates` (via `GetAllBlocksHandler` / `GetAllTemplatesHandler` in diff --git a/docs/decisions.md b/docs/decisions.md index 1a9c1f7d7..e3e6ea615 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -563,3 +563,44 @@ action of the Step 2 deletion PR merge** (not before — `main` moves until then cut. Not cut this session — `main` still carries Blazor and will advance before the removal PR. + +## 2026-07-11 — Post-commit side effects run on `CancellationToken.None` (generalized from #251 to #254) + +Audit #22 (adversarial-reviewer) found ~20 command handlers threading the request `cancellationToken` +into work that runs **after** `SaveChangesAsync` commits — the post-commit `WriteAsync` enqueue that +rebuilds/refreshes the affected entity, `mediator.Publish`, search-index reindex, cache refresh. A late +HTTP-client disconnect cancels that token, so the *already-committed* mutation throws on the way out +**and silently drops its side effect** (the playout rebuild is never queued → the persisted edit never +takes visible effect until a manual Reset). #251 fixed this for the deco handlers; #254 generalizes the +policy across the codebase. + +**Decision.** Once a mutation has committed, the *entire* compensating side effect — enqueues, publishes, +reindexes, cache refreshes, and any post-commit lookup that **gates** one of those enqueues — runs on +`CancellationToken.None`. The commit is the point of no return; past it the side effect must not be +half-abortable. Full convention + the two boundaries in `docs/api-conventions.md` §7a. + +**Scope of the #254 sweep (this PR).** Swept the single-`SaveChanges` handlers under `MediaCollections/`, +`ProgramSchedules/`, `Playouts/`, `Channels/` (20 handlers). Deliberately **excluded**: +- **`BuildPlayoutHandler`** — a background/worker handler; its token is the worker shutdown token, not a + client-disconnect token, so its downstream enqueues *correctly* honor cancellation. +- **`UpdateFFmpegSettingsHandler` + the two `Configuration/` settings handlers** — they commit via several + sequential `IConfigElementRepository.Upsert` calls with an interleaved enqueue; "when is it committed" + is a partial-commit-under-cancellation question broader than the clean single-`SaveChanges` F4 pattern. + Left for a separate follow-up. +- **Response-projection reloads** (`ReplaceProgramScheduleItemsHandler` / `AddProgramScheduleItemHandler` + post-commit graph reload that builds the *returned* view model) keep the request token — a cancelled + response after a durable commit + `None`-enqueue loses nothing. +- Handlers a no-token `WriteAsync()` already makes behaviorally correct (`default` == `None`) were left + alone (explicit-`None` there is cosmetic). + +Also folded in the two other #254 items on the same handlers: the channel-guide `{number}.xml` delete in +`DeleteChannelHandler`/`DeletePlayoutHandler` now routes through `IFileSystem.File.Delete` (observable +under `MockFileSystem`) **before** the commit (a post-commit delete orphans the xml on a crash; the guide +xml is regenerable on demand, so a pre-commit delete is the safe ordering), and +`ReplacePlayoutAlternateScheduleItemsHandler` now rejects an empty item list in the handler (not only at +the controller pre-guard) so a direct caller can't trip the `Max()`-on-empty crash. + +**Coordination note for #253 PR2–PR4.** Those PRs add `Version++` (pre-commit) to the mutating handlers of +the versioned aggregates — several of which this sweep also touched (post-commit token, a different line +region). Low git-conflict risk, but merge `main` in and expect to see the `CancellationToken.None` +convention already present on the post-commit enqueues. From 6055bd368738e82c2e5ccdd259a45940cb51209f Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 11 Jul 2026 17:49:16 +0200 Subject: [PATCH 02/12] fix(app): fold Create/DeleteSmartCollection into the post-commit None sweep (#254 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Independent adversarial review of PR #266 found two sibling handlers with the identical post-commit `_smartCollectionCache.Refresh(cancellationToken)` pattern that the sweep missed (only UpdateSmartCollection was caught). Same audit#22 F4 class: a late client-disconnect after the commit lands would throw and leave the in-memory smart-collection cache stale vs the committed DB. → CancellationToken.None. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Commands/CreateSmartCollectionHandler.cs | 5 ++++- .../Commands/DeleteSmartCollectionHandler.cs | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/ErsatzTV.Application/MediaCollections/Commands/CreateSmartCollectionHandler.cs b/ErsatzTV.Application/MediaCollections/Commands/CreateSmartCollectionHandler.cs index 5a9783c45..33ed35131 100644 --- a/ErsatzTV.Application/MediaCollections/Commands/CreateSmartCollectionHandler.cs +++ b/ErsatzTV.Application/MediaCollections/Commands/CreateSmartCollectionHandler.cs @@ -42,7 +42,10 @@ public class CreateSmartCollectionHandler : await dbContext.SmartCollections.AddAsync(smartCollection, cancellationToken); await dbContext.SaveChangesAsync(cancellationToken); _searchTargets.SearchTargetsChanged(); - await _smartCollectionCache.Refresh(cancellationToken); + + // post-commit side effect runs on CancellationToken.None so a late request cancellation + // can't abort it after the commit landed (#254) + await _smartCollectionCache.Refresh(CancellationToken.None); return ProjectToViewModel(smartCollection); } diff --git a/ErsatzTV.Application/MediaCollections/Commands/DeleteSmartCollectionHandler.cs b/ErsatzTV.Application/MediaCollections/Commands/DeleteSmartCollectionHandler.cs index 7fdea22d6..2b6e849a8 100644 --- a/ErsatzTV.Application/MediaCollections/Commands/DeleteSmartCollectionHandler.cs +++ b/ErsatzTV.Application/MediaCollections/Commands/DeleteSmartCollectionHandler.cs @@ -48,7 +48,10 @@ public class DeleteSmartCollectionHandler : IRequestHandler Date: Sat, 11 Jul 2026 17:54:21 +0200 Subject: [PATCH 03/12] fix(235-A): normalize async-op error contracts on Maintenance + Troubleshoot controllers (#235) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slice A of the async-op API contract normalization. MaintenanceController: - EmptyTrash error path: was 500 text/plain (error.ToString()); now maps the BaseError Left through ApiResults.ToErrorResult() -> 404 (NotFoundError) / 422 ProblemDetails. Success stays 200 OkResult. Added ProducesResponseType 200 + 422. - CleanArtwork: fire-and-forget enqueue of DeleteOrphanedArtwork was a silent 200; now returns 202 Accepted (AcceptedResult) since it queues background work. Added ProducesResponseType 202. (Controller does not derive from ControllerBase, so results are built directly as before.) TroubleshootController.TroubleshootPlayback (GET|HEAD /api/troubleshoot/playback.m3u8): - Two bare body-less NotFound() call sites conflated "not found" with "prepare/ playback failure". Both now return a ProblemDetails body: * prepare-failure (result.IsLeft): mapped through error.ToErrorResult() -> 404 for NotFoundError (unknown media item/channel) else 422 for a validation BaseError. * terminal fall-through (prepare ok but no playable output): kept 404 with a distinguishing ApiResults.NotFoundProblem(...) detail. - Added ProducesResponseType 404 + 422 (409 already present). Consumer check: the SPA (PlaybackTroubleshootingScreen) feeds the playback.m3u8 URL straight to hls.js via HlsPlayer, which never inspects the HTTP status code — playback state is surfaced via the separate /api/troubleshoot/playback/status poll. So the 404->422 split for the validation subcase is safe; no player code branches on the status code. Tests: MaintenanceControllerTests (200/422/202 + enqueue assertion), TroubleshootControllerTests (prepare 404 NotFoundError, 422 validation). All green; Api error-metadata/contract/security scans still pass. Note: OpenAPI artifacts (v1.json / v1.d.ts) intentionally NOT regenerated here — the orchestrator regenerates once after all #235 slices merge. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Controllers/MaintenanceControllerTests.cs | 67 +++++++++++++++++++ .../TroubleshootControllerTests.cs | 50 ++++++++++++++ .../Controllers/Api/MaintenanceController.cs | 13 ++-- .../Controllers/Api/TroubleshootController.cs | 19 +++++- 4 files changed, 139 insertions(+), 10 deletions(-) create mode 100644 ErsatzTV.Tests/Controllers/MaintenanceControllerTests.cs diff --git a/ErsatzTV.Tests/Controllers/MaintenanceControllerTests.cs b/ErsatzTV.Tests/Controllers/MaintenanceControllerTests.cs new file mode 100644 index 000000000..35c8416d1 --- /dev/null +++ b/ErsatzTV.Tests/Controllers/MaintenanceControllerTests.cs @@ -0,0 +1,67 @@ +using System.Threading.Channels; +using ErsatzTV.Application; +using ErsatzTV.Application.Maintenance; +using ErsatzTV.Controllers.Api; +using ErsatzTV.Core; +using LanguageExt; +using MediatR; +using Microsoft.AspNetCore.Mvc; +using NSubstitute; +using NUnit.Framework; +using Shouldly; +using static LanguageExt.Prelude; + +namespace ErsatzTV.Tests.Controllers; + +[TestFixture] +public class MaintenanceControllerTests +{ + private MaintenanceController _controller = null!; + private IMediator _mediator = null!; + private Channel _workerChannel = null!; + + [SetUp] + public void SetUp() + { + _mediator = Substitute.For(); + _workerChannel = Channel.CreateUnbounded(); + _controller = new MaintenanceController(_mediator, _workerChannel.Writer); + } + + [Test] + public async Task EmptyTrash_Should_Return_200_On_Success() + { + _mediator.Send(Arg.Any()) + .Returns(Right(unit)); + + IActionResult result = await _controller.EmptyTrash(); + + result.ShouldBeOfType(); + await _mediator.Received(1).Send(Arg.Any()); + } + + [Test] + public async Task EmptyTrash_Should_Return_422_ProblemDetails_On_Error() + { + _mediator.Send(Arg.Any()) + .Returns(Left(BaseError.New("Failed to empty trash"))); + + IActionResult result = await _controller.EmptyTrash(); + + var unprocessable = result.ShouldBeOfType(); + var problem = unprocessable.Value.ShouldBeOfType(); + problem.Status.ShouldBe(422); + problem.Detail.ShouldBe("Failed to empty trash"); + } + + [Test] + public async Task CleanArtwork_Should_Return_202_And_Enqueue_DeleteOrphanedArtwork() + { + IActionResult result = await _controller.CleanArtwork(CancellationToken.None); + + result.ShouldBeOfType(); + + _workerChannel.Reader.TryRead(out IBackgroundServiceRequest? enqueued).ShouldBeTrue(); + enqueued.ShouldBeOfType(); + } +} diff --git a/ErsatzTV.Tests/Controllers/TroubleshootControllerTests.cs b/ErsatzTV.Tests/Controllers/TroubleshootControllerTests.cs index e04ac59cf..31d470256 100644 --- a/ErsatzTV.Tests/Controllers/TroubleshootControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/TroubleshootControllerTests.cs @@ -12,6 +12,7 @@ using ErsatzTV.Controllers.Api.Requests; using ErsatzTV.Core; using ErsatzTV.Core.Api.Troubleshooting; using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Interfaces.FFmpeg; using ErsatzTV.Core.Interfaces.Locking; using ErsatzTV.Core.Interfaces.Repositories; using ErsatzTV.Core.Interfaces.Troubleshooting; @@ -274,6 +275,55 @@ public class TroubleshootControllerTests .Send(Arg.Any(), Arg.Any()); } + [Test] + public async Task TroubleshootPlayback_Should_Return_404_ProblemDetails_When_Prepare_Not_Found() + { + _entityLocker.IsTroubleshootingPlaybackLocked().Returns(false); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Either.Left( + new ErsatzTV.Core.Errors.NotFoundError("no such media item"))); + + IActionResult result = await _controller.TroubleshootPlayback( + mediaItem: 999, + channel: 0, + ffmpegProfile: 1, + StreamingMode.HttpLiveStreamingSegmenter, + watermark: [], + graphicsElement: [], + streamSelector: string.Empty, + subtitleId: null, + seekSeconds: 0, + start: null, + CancellationToken.None); + + var notFound = result.ShouldBeOfType(); + notFound.Value.ShouldBeOfType().Status.ShouldBe(404); + } + + [Test] + public async Task TroubleshootPlayback_Should_Return_422_ProblemDetails_When_Prepare_Fails_Validation() + { + _entityLocker.IsTroubleshootingPlaybackLocked().Returns(false); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Either.Left(BaseError.New("unable to prepare"))); + + IActionResult result = await _controller.TroubleshootPlayback( + mediaItem: 1, + channel: 0, + ffmpegProfile: 1, + StreamingMode.HttpLiveStreamingSegmenter, + watermark: [], + graphicsElement: [], + streamSelector: string.Empty, + subtitleId: null, + seekSeconds: 0, + start: null, + CancellationToken.None); + + var unprocessable = result.ShouldBeOfType(); + unprocessable.Value.ShouldBeOfType().Status.ShouldBe(422); + } + [Test] public async Task GetPlaybackStatus_Should_Report_Idle_When_No_Result_And_Unlocked() { diff --git a/ErsatzTV/Controllers/Api/MaintenanceController.cs b/ErsatzTV/Controllers/Api/MaintenanceController.cs index b6009e32a..2093207b7 100644 --- a/ErsatzTV/Controllers/Api/MaintenanceController.cs +++ b/ErsatzTV/Controllers/Api/MaintenanceController.cs @@ -2,6 +2,7 @@ using System.Threading.Channels; using ErsatzTV.Application; using ErsatzTV.Application.Maintenance; using ErsatzTV.Core; +using ErsatzTV.Extensions; using MediatR; using Microsoft.AspNetCore.Mvc; @@ -23,17 +24,14 @@ public class MaintenanceController(IMediator mediator, ChannelWriter EmptyTrash() { Either result = await mediator.Send(new EmptyTrash()); foreach (BaseError error in result.LeftToSeq()) { - return new ContentResult - { - StatusCode = StatusCodes.Status500InternalServerError, - Content = error.ToString(), - ContentType = "text/plain" - }; + return error.ToErrorResult(); } return new OkResult(); @@ -42,9 +40,10 @@ public class MaintenanceController(IMediator mediator, ChannelWriter CleanArtwork(CancellationToken cancellationToken) { await workerChannel.WriteAsync(new DeleteOrphanedArtwork(), cancellationToken); - return new OkResult(); + return new AcceptedResult(); } } diff --git a/ErsatzTV/Controllers/Api/TroubleshootController.cs b/ErsatzTV/Controllers/Api/TroubleshootController.cs index 129c077f5..7a523f3f7 100644 --- a/ErsatzTV/Controllers/Api/TroubleshootController.cs +++ b/ErsatzTV/Controllers/Api/TroubleshootController.cs @@ -114,7 +114,9 @@ public class TroubleshootController( [Tags("Troubleshooting")] [EndpointSummary("Start a troubleshooting playback session")] [EndpointGroupName("general")] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task TroubleshootPlayback( [FromQuery] int mediaItem, @@ -174,9 +176,15 @@ public class TroubleshootController( Optional(start)), cancellationToken); - if (result.IsLeft) + // Distinguish "prepare failed" from the later "no playable output" fall-through: map the + // handler's BaseError through the standard helper (404 for NotFoundError — e.g. an unknown + // media item/channel — else 422 for a validation failure) with a ProblemDetails body, + // instead of a bare body-less 404. The SPA feeds this URL straight to hls.js (HlsPlayer) + // and never inspects the status code — failures surface via the /status poll — so the + // 404→422 split for validation errors is safe. + foreach (BaseError error in result.LeftToSeq()) { - return NotFound(); + return error.ToErrorResult(); } // Prepare returned a process, so the handler holds the troubleshooting lock now @@ -273,7 +281,12 @@ public class TroubleshootController( } } - return NotFound(); + // Terminal fall-through: Prepare succeeded but no playable output was produced (playback + // failed to start, was cancelled, or the segmenter never wrote segments). Keep the 404 status + // the SPA player already tolerates, but attach a distinguishing ProblemDetails body rather + // than a bare NotFound() so the response is self-describing. + return ApiResults.NotFoundProblem( + "Troubleshooting playback did not produce any output. It may have failed to start or been cancelled."); } [HttpHead("api/troubleshoot/playback/archive")] From d02f953922e8d30d3265e040e7404107456e62ac Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 11 Jul 2026 17:57:41 +0200 Subject: [PATCH 04/12] fix(235-F7): release leaked Trakt lock on worker shutdown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The global Trakt lock is acquired by SchedulerService.RefreshTraktLists / MatchTraktLists (and TraktController) and released only when the *terminal* message of a batch — the one carrying Unlock: true (list == traktLists.Last()) — is processed by WorkerService, whose handler (AddTraktListHandler / MatchTraktListItemsHandler) calls IEntityLocker.UnlockTrakt() in a finally. WorkerService.ExecuteAsync breaks out of the read loop on stoppingToken.IsCancellationRequested (and exits on channel completion / reader cancellation) BEFORE processing the next message. If shutdown lands after a batch is enqueued but before its terminal Unlock: true message is handled, UnlockTrakt() never runs and the in-memory Trakt lock leaks for the rest of the process lifetime (subsequent Trakt operations 409 forever). Fix (option a): make the batch-release loss-tolerant with a compensating release in a finally around the read loop — if the Trakt lock is still held when the worker stops, release it. Chosen over tracking pending ownership (b) because the lock is a global singleton and WorkerService is its sole batch-release site, so "held at shutdown" unambiguously means "the terminal release was lost"; covers all three exit paths (break / channel completion / cancellation) in one place. Same lock-lifecycle class as #231/#233/#234. Regression test: WorkerServiceTests gates the first (non-terminal) batch message on the stopping token, then StopAsync-cancels so the worker breaks before the terminal Unlock: true message — asserts the lock is released and the terminal message was never processed. Proven non-vacuous: inverting the finally condition fails the test. Backend-only; no controller/DTO/SPA/OpenAPI impact. Co-Authored-By: Claude Opus 4.8 (1M context) --- ErsatzTV.Tests/Services/WorkerServiceTests.cs | 101 ++++++++++++++++++ ErsatzTV/Services/WorkerService.cs | 18 ++++ 2 files changed, 119 insertions(+) create mode 100644 ErsatzTV.Tests/Services/WorkerServiceTests.cs diff --git a/ErsatzTV.Tests/Services/WorkerServiceTests.cs b/ErsatzTV.Tests/Services/WorkerServiceTests.cs new file mode 100644 index 000000000..96c6cd404 --- /dev/null +++ b/ErsatzTV.Tests/Services/WorkerServiceTests.cs @@ -0,0 +1,101 @@ +using System.Threading.Channels; +using ErsatzTV.Application; +using ErsatzTV.Application.MediaCollections; +using ErsatzTV.Core; +using ErsatzTV.Core.Interfaces.Locking; +using ErsatzTV.Services; +using LanguageExt; +using MediatR; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; +using NUnit.Framework; +using Shouldly; +using Unit = LanguageExt.Unit; + +namespace ErsatzTV.Tests.Services; + +[TestFixture] +public class WorkerServiceTests +{ + // F7 regression (issue #235): the global Trakt lock is acquired by SchedulerService and released + // only when the *terminal* (Unlock: true) message of a batch is processed by WorkerService. If the + // worker stops before reaching that terminal message (shutdown break / channel completion / + // cancellation), the release never fires and the in-memory Trakt lock leaks for the life of the + // process. WorkerService must release a held Trakt lock as a compensating action on shutdown. + [Test] + public async Task Should_Release_Held_Trakt_Lock_When_Worker_Stops_Before_Terminal_Message() + { + var channel = Channel.CreateUnbounded(); + + // A Trakt batch: only the terminal message carries Unlock: true. If the worker never processes + // it, the handler-side UnlockTrakt() never runs. + AddTraktList nonTerminal = AddTraktList.Existing("user", "list-1", false); + AddTraktList terminal = AddTraktList.Existing("user", "list-2", true); + await channel.Writer.WriteAsync(nonTerminal); + await channel.Writer.WriteAsync(terminal); + + // Stateful stand-in for the singleton EntityLocker: the lock starts held (a batch acquired it). + var traktLocked = 1; + var locker = Substitute.For(); + locker.IsTraktLocked().Returns(_ => Volatile.Read(ref traktLocked) == 1); + locker.UnlockTrakt().Returns(_ => Interlocked.Exchange(ref traktLocked, 0) == 1); + + // Gate: processing the first (non-terminal) message parks on the stopping token, guaranteeing + // the worker never reaches the terminal (Unlock: true) message before it is stopped. + var firstSeen = new TaskCompletionSource(); + var processed = new List(); + var mediator = Substitute.For(); + mediator.Send(Arg.Any(), Arg.Any()) + .Returns(async call => + { + lock (processed) + { + processed.Add(call.Arg()); + } + + firstSeen.TrySetResult(); + + // Block on the stopping token so the loop parks here until StopAsync cancels it. + await Task.Delay(Timeout.Infinite, call.Arg()); + return (Either)Unit.Default; + }); + + var provider = Substitute.For(); + provider.GetService(typeof(IMediator)).Returns(mediator); + var scope = Substitute.For(); + scope.ServiceProvider.Returns(provider); + var scopeFactory = Substitute.For(); + scopeFactory.CreateScope().Returns(scope); + + var worker = new WorkerService( + channel.Reader, + scopeFactory, + locker, + NullLogger.Instance); + + await worker.StartAsync(CancellationToken.None); + + // Wait until the first message is actively being processed (parked on the stopping token). + await firstSeen.Task.WaitAsync(TimeSpan.FromSeconds(5)); + + // Shut the worker down: cancels the stopping token -> parked Delay throws -> loop breaks + // before the terminal message is ever processed. + await worker.StopAsync(CancellationToken.None); + + // The compensating release must have fired even though the terminal message was never handled. + locker.IsTraktLocked().ShouldBeFalse(); + locker.Received(1).UnlockTrakt(); + + // Prove the leak scenario is genuine: we stopped after the non-terminal message but before the + // terminal (Unlock: true) one, so the handler-side release could not have run. + List seen; + lock (processed) + { + seen = processed.ToList(); + } + + seen.ShouldContain(nonTerminal); + seen.ShouldNotContain(terminal); + } +} diff --git a/ErsatzTV/Services/WorkerService.cs b/ErsatzTV/Services/WorkerService.cs index 5e3ebfd00..1cefb8eb2 100644 --- a/ErsatzTV/Services/WorkerService.cs +++ b/ErsatzTV/Services/WorkerService.cs @@ -9,6 +9,7 @@ using ErsatzTV.Application.MediaCollections; using ErsatzTV.Application.Playouts; using ErsatzTV.Application.Subtitles; using ErsatzTV.Core; +using ErsatzTV.Core.Interfaces.Locking; using MediatR; namespace ErsatzTV.Services; @@ -16,16 +17,19 @@ namespace ErsatzTV.Services; public class WorkerService : BackgroundService { private readonly ChannelReader _channel; + private readonly IEntityLocker _entityLocker; private readonly ILogger _logger; private readonly IServiceScopeFactory _serviceScopeFactory; public WorkerService( ChannelReader channel, IServiceScopeFactory serviceScopeFactory, + IEntityLocker entityLocker, ILogger logger) { _channel = channel; _serviceScopeFactory = serviceScopeFactory; + _entityLocker = entityLocker; _logger = logger; } @@ -143,5 +147,19 @@ public class WorkerService : BackgroundService { _logger.LogInformation("Worker service shutting down"); } + finally + { + // The global Trakt lock is acquired by SchedulerService/TraktController and released only + // when the *terminal* (Unlock: true) message of a batch is processed here. If this loop + // stops before reaching that message - shutdown break above, channel completion, or the + // reader throwing on cancellation - the release never fires and the in-memory Trakt lock + // leaks for the remaining life of the process (subsequent Trakt operations 409 forever). + // Make the batch-release loss-tolerant with a compensating release on worker shutdown. + if (_entityLocker.IsTraktLocked()) + { + _logger.LogDebug("Releasing held Trakt lock during worker shutdown"); + _entityLocker.UnlockTrakt(); + } + } } } From 9b73b62527c111c3987503f2bb945d1012ce8932 Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 11 Jul 2026 17:58:15 +0200 Subject: [PATCH 05/12] =?UTF-8?q?feat(235):=20async-op=20API=20contract=20?= =?UTF-8?q?normalization=20=E2=80=94=20playouts=20slice=20C=20(#235)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slice C of the async-op contract normalization: - channel reset (POST /api/channels/{channelNumber}/playout/reset) now returns 202 Accepted (was 200 Ok) — it only queues a background rebuild - reset-all (POST /api/playouts/reset-all) still 202 but now returns a ResetAllPlayoutsResponseModel body reporting QueuedPlayoutIds / SkippedLocked / SkippedUnsupported instead of silently swallowing skips; handler returns a new ResetAllPlayoutsResult record - single-playout GET (GET /api/playouts/{id}) now exposes IsLocked on PlayoutResponseModel, set from IEntityLocker.IsPlayoutLocked mirroring the list projection — gives a polling client the lock flag Tests: channel reset asserts 202; reset-all asserts 202 + skipped-body shape; single GET asserts IsLocked; new ResetAllPlayoutsHandlerTests (in-memory SQLite) asserts locked/ExternalJson/None land in skipped lists and eligible playouts in queued. docs/api-conventions.md §3a updated. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Playouts/Commands/ResetAllPlayouts.cs | 2 +- .../Commands/ResetAllPlayoutsHandler.cs | 27 +++++- .../Playouts/ResetAllPlayoutsResult.cs | 6 ++ .../Api/Playouts/PlayoutResponseModel.cs | 9 +- .../Playouts/ResetAllPlayoutsResponseModel.cs | 7 ++ .../Playouts/ResetAllPlayoutsHandlerTests.cs | 82 +++++++++++++++++++ .../Controllers/ChannelControllerTests.cs | 4 +- .../Controllers/PlayoutControllerTests.cs | 30 ++++++- ErsatzTV/Controllers/Api/ChannelController.cs | 4 +- ErsatzTV/Controllers/Api/PlayoutController.cs | 29 ++++--- docs/api-conventions.md | 17 +++- 11 files changed, 188 insertions(+), 29 deletions(-) create mode 100644 ErsatzTV.Application/Playouts/ResetAllPlayoutsResult.cs create mode 100644 ErsatzTV.Core/Api/Playouts/ResetAllPlayoutsResponseModel.cs create mode 100644 ErsatzTV.Tests/Application/Playouts/ResetAllPlayoutsHandlerTests.cs diff --git a/ErsatzTV.Application/Playouts/Commands/ResetAllPlayouts.cs b/ErsatzTV.Application/Playouts/Commands/ResetAllPlayouts.cs index dbf41139d..79967ab48 100644 --- a/ErsatzTV.Application/Playouts/Commands/ResetAllPlayouts.cs +++ b/ErsatzTV.Application/Playouts/Commands/ResetAllPlayouts.cs @@ -1,3 +1,3 @@ namespace ErsatzTV.Application.Playouts; -public record ResetAllPlayouts : IRequest; +public record ResetAllPlayouts : IRequest; diff --git a/ErsatzTV.Application/Playouts/Commands/ResetAllPlayoutsHandler.cs b/ErsatzTV.Application/Playouts/Commands/ResetAllPlayoutsHandler.cs index c99a127f6..e997bedb7 100644 --- a/ErsatzTV.Application/Playouts/Commands/ResetAllPlayoutsHandler.cs +++ b/ErsatzTV.Application/Playouts/Commands/ResetAllPlayoutsHandler.cs @@ -11,33 +11,49 @@ public class ResetAllPlayoutsHandler( IEntityLocker locker, ChannelWriter channel, IDbContextFactory dbContextFactory) - : IRequestHandler + : IRequestHandler { - public async Task Handle(ResetAllPlayouts request, CancellationToken cancellationToken) + public async Task Handle( + ResetAllPlayouts request, + CancellationToken cancellationToken) { await using TvContext dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); + var queued = new List(); + var skippedLocked = new List(); + var skippedUnsupported = new List(); + foreach (Playout playout in await dbContext.Playouts.ToListAsync(cancellationToken)) { switch (playout.ScheduleKind) { case PlayoutScheduleKind.Classic: - if (!locker.IsPlayoutLocked(playout.Id)) + if (locker.IsPlayoutLocked(playout.Id)) + { + skippedLocked.Add(playout.Id); + } + else { await channel.WriteAsync( new BuildPlayout(playout.Id, PlayoutBuildMode.Refresh), cancellationToken); + queued.Add(playout.Id); } break; case PlayoutScheduleKind.Block: case PlayoutScheduleKind.Sequential: case PlayoutScheduleKind.Scripted: - if (!locker.IsPlayoutLocked(playout.Id)) + if (locker.IsPlayoutLocked(playout.Id)) + { + skippedLocked.Add(playout.Id); + } + else { await channel.WriteAsync( new BuildPlayout(playout.Id, PlayoutBuildMode.Reset), cancellationToken); + queued.Add(playout.Id); } break; @@ -45,8 +61,11 @@ public class ResetAllPlayoutsHandler( case PlayoutScheduleKind.None: default: // external json cannot be reset + skippedUnsupported.Add(playout.Id); continue; } } + + return new ResetAllPlayoutsResult(queued, skippedLocked, skippedUnsupported); } } diff --git a/ErsatzTV.Application/Playouts/ResetAllPlayoutsResult.cs b/ErsatzTV.Application/Playouts/ResetAllPlayoutsResult.cs new file mode 100644 index 000000000..0d5fd9633 --- /dev/null +++ b/ErsatzTV.Application/Playouts/ResetAllPlayoutsResult.cs @@ -0,0 +1,6 @@ +namespace ErsatzTV.Application.Playouts; + +public record ResetAllPlayoutsResult( + List QueuedPlayoutIds, + List SkippedLocked, + List SkippedUnsupported); diff --git a/ErsatzTV.Core/Api/Playouts/PlayoutResponseModel.cs b/ErsatzTV.Core/Api/Playouts/PlayoutResponseModel.cs index 63b08b96b..3439da4f9 100644 --- a/ErsatzTV.Core/Api/Playouts/PlayoutResponseModel.cs +++ b/ErsatzTV.Core/Api/Playouts/PlayoutResponseModel.cs @@ -14,7 +14,8 @@ public record PlayoutResponseModel( TimeSpan? DailyRebuildTime, PlayoutBuildStatusResponseModel? BuildStatus, int? DecoId, - string? DecoName) + string? DecoName, + bool IsLocked) { public static PlayoutResponseModel From( int id, @@ -27,7 +28,8 @@ public record PlayoutResponseModel( TimeSpan? dailyRebuildTime, PlayoutBuildStatusResponseModel? buildStatus, int? decoId, - string? decoName) => + string? decoName, + bool isLocked) => new( id, scheduleKind, @@ -39,5 +41,6 @@ public record PlayoutResponseModel( dailyRebuildTime, buildStatus, decoId, - decoName); + decoName, + isLocked); } diff --git a/ErsatzTV.Core/Api/Playouts/ResetAllPlayoutsResponseModel.cs b/ErsatzTV.Core/Api/Playouts/ResetAllPlayoutsResponseModel.cs new file mode 100644 index 000000000..830b2bb15 --- /dev/null +++ b/ErsatzTV.Core/Api/Playouts/ResetAllPlayoutsResponseModel.cs @@ -0,0 +1,7 @@ +#nullable enable +namespace ErsatzTV.Core.Api.Playouts; + +public record ResetAllPlayoutsResponseModel( + List QueuedPlayoutIds, + List SkippedLocked, + List SkippedUnsupported); diff --git a/ErsatzTV.Tests/Application/Playouts/ResetAllPlayoutsHandlerTests.cs b/ErsatzTV.Tests/Application/Playouts/ResetAllPlayoutsHandlerTests.cs new file mode 100644 index 000000000..e7b6ffab1 --- /dev/null +++ b/ErsatzTV.Tests/Application/Playouts/ResetAllPlayoutsHandlerTests.cs @@ -0,0 +1,82 @@ +using System.Collections.Generic; +using System.Threading; +using System.Threading.Channels; +using System.Threading.Tasks; +using ErsatzTV.Application; +using ErsatzTV.Application.Playouts; +using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Interfaces.Locking; +using ErsatzTV.Core.Scheduling; +using ErsatzTV.Infrastructure.Data; +using ErsatzTV.Tests.Support; +using NSubstitute; +using NUnit.Framework; +using Shouldly; +using Channel = System.Threading.Channels.Channel; + +namespace ErsatzTV.Tests.Application.Playouts; + +[TestFixture] +public class ResetAllPlayoutsHandlerTests +{ + private InMemoryTvContext _db = null!; + private Channel _worker = null!; + private IEntityLocker _entityLocker = null!; + + [SetUp] + public async Task SetUp() + { + _db = await InMemoryTvContext.CreateAsync(); + _worker = Channel.CreateUnbounded(); + _entityLocker = Substitute.For(); + } + + [TearDown] + public async Task TearDown() => await _db.DisposeAsync(); + + private ResetAllPlayoutsHandler CreateHandler() => + new(_entityLocker, _worker.Writer, _db.Factory); + + private async Task SeedPlayout(PlayoutScheduleKind kind) + { + await using TvContext context = _db.CreateContext(); + var playout = new Playout { ChannelId = 0, ScheduleKind = kind }; + context.Playouts.Add(playout); + await context.SaveChangesAsync(); + return playout.Id; + } + + [Test] + public async Task Handle_Should_Queue_Eligible_And_Report_Skipped() + { + int classicId = await SeedPlayout(PlayoutScheduleKind.Classic); + int blockId = await SeedPlayout(PlayoutScheduleKind.Block); + int lockedId = await SeedPlayout(PlayoutScheduleKind.Sequential); + int externalJsonId = await SeedPlayout(PlayoutScheduleKind.ExternalJson); + int noneId = await SeedPlayout(PlayoutScheduleKind.None); + + _entityLocker.IsPlayoutLocked(lockedId).Returns(true); + + ResetAllPlayoutsResult result = + await CreateHandler().Handle(new ResetAllPlayouts(), CancellationToken.None); + + // eligible, unlocked playouts are queued + result.QueuedPlayoutIds.ShouldBe(new List { classicId, blockId }, ignoreOrder: true); + + // locked playout lands in SkippedLocked, not queued + result.SkippedLocked.ShouldBe(new List { lockedId }); + + // ExternalJson + None land in SkippedUnsupported + result.SkippedUnsupported.ShouldBe(new List { externalJsonId, noneId }, ignoreOrder: true); + + // exactly one BuildPlayout message per queued playout was enqueued + var enqueued = new List(); + while (_worker.Reader.TryRead(out IBackgroundServiceRequest? request)) + { + var build = request.ShouldBeOfType(); + enqueued.Add(build.PlayoutId); + } + + enqueued.ShouldBe(new List { classicId, blockId }, ignoreOrder: true); + } +} diff --git a/ErsatzTV.Tests/Controllers/ChannelControllerTests.cs b/ErsatzTV.Tests/Controllers/ChannelControllerTests.cs index 67b84c74e..5e1deb4e3 100644 --- a/ErsatzTV.Tests/Controllers/ChannelControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/ChannelControllerTests.cs @@ -407,7 +407,7 @@ public class ChannelControllerTests IActionResult result = await _controller.ResetPlayout("5", mode: null, CancellationToken.None); - result.ShouldBeOfType(); + result.ShouldBeOfType().StatusCode.ShouldBe(202); _workerChannel.Reader.TryRead(out IBackgroundServiceRequest? request).ShouldBeTrue(); var buildPlayout = request.ShouldBeOfType(); buildPlayout.PlayoutId.ShouldBe(9); @@ -436,7 +436,7 @@ public class ChannelControllerTests IActionResult result = await _controller.ResetPlayout("5", PlayoutBuildMode.Continue, CancellationToken.None); - result.ShouldBeOfType(); + result.ShouldBeOfType().StatusCode.ShouldBe(202); _workerChannel.Reader.TryRead(out IBackgroundServiceRequest? request).ShouldBeTrue(); request.ShouldBeOfType().Mode.ShouldBe(PlayoutBuildMode.Continue); await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); diff --git a/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs b/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs index 69b5171ef..0fc9e3bc5 100644 --- a/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs @@ -527,6 +527,21 @@ public class PlayoutControllerTests result.ShouldBeOfType().Value.ShouldBe(ToResponse(vm)); } + [Test] + public async Task GetById_Should_Expose_IsLocked_From_EntityLocker() + { + PlayoutNameViewModel vm = MakePlayout(9); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(vm)); + _entityLocker.IsPlayoutLocked(9).Returns(true); + + IActionResult result = await _controller.GetById(9, CancellationToken.None); + + var body = result.ShouldBeOfType().Value.ShouldBeOfType(); + body.IsLocked.ShouldBeTrue(); + body.ShouldBe(ToResponse(vm, isLocked: true)); + } + [Test] public async Task GetById_Should_Return_404_For_None_With_ProblemDetails() { @@ -693,9 +708,17 @@ public class PlayoutControllerTests [Test] public async Task ResetAll_Should_Return_202_And_Send_Command() { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(new ResetAllPlayoutsResult([1, 2], [3], [4])); + IActionResult result = await _controller.ResetAll(CancellationToken.None); - result.ShouldBeOfType(); + var accepted = result.ShouldBeOfType(); + accepted.StatusCode.ShouldBe(202); + var body = accepted.Value.ShouldBeOfType(); + body.QueuedPlayoutIds.ShouldBe(new List { 1, 2 }); + body.SkippedLocked.ShouldBe(new List { 3 }); + body.SkippedUnsupported.ShouldBe(new List { 4 }); await _mediator.Received(1).Send(Arg.Any(), Arg.Any()); } @@ -1286,7 +1309,7 @@ public class PlayoutControllerTests null, null); - private static PlayoutResponseModel ToResponse(PlayoutNameViewModel vm) => + private static PlayoutResponseModel ToResponse(PlayoutNameViewModel vm, bool isLocked = false) => PlayoutResponseModel.From( vm.PlayoutId, vm.ScheduleKind, @@ -1303,7 +1326,8 @@ public class PlayoutControllerTests vm.BuildStatus.Success, vm.BuildStatus.Message), vm.DecoId, - vm.DecoName); + vm.DecoName, + isLocked); private static void ShouldHaveActionRoute(string actionName, string httpMethod, string route) { diff --git a/ErsatzTV/Controllers/Api/ChannelController.cs b/ErsatzTV/Controllers/Api/ChannelController.cs index e70f1a833..d2357e9e7 100644 --- a/ErsatzTV/Controllers/Api/ChannelController.cs +++ b/ErsatzTV/Controllers/Api/ChannelController.cs @@ -211,7 +211,7 @@ public class ChannelController( "progress) and all other playout kinds use Reset (rebuild from scratch), matching the Blazor UI. " + "Pass mode to force a specific PlayoutBuildMode.")] [EndpointGroupName("general")] - [ProducesResponseType(StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status202Accepted)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] public async Task ResetPlayout( @@ -233,7 +233,7 @@ public class ChannelController( PlayoutBuildMode buildMode = mode ?? await DefaultResetMode(playoutId, cancellationToken); await workerChannel.WriteAsync(new BuildPlayout(playoutId, buildMode), cancellationToken); - return new OkResult(); + return new AcceptedResult(); } return ApiResults.NotFoundProblem(); diff --git a/ErsatzTV/Controllers/Api/PlayoutController.cs b/ErsatzTV/Controllers/Api/PlayoutController.cs index a0bff515f..c023135d7 100644 --- a/ErsatzTV/Controllers/Api/PlayoutController.cs +++ b/ErsatzTV/Controllers/Api/PlayoutController.cs @@ -69,7 +69,7 @@ public class PlayoutController(IMediator mediator, IEntityLocker entityLocker) : public async Task GetById(int id, CancellationToken cancellationToken) { Option result = await mediator.Send(new GetPlayoutById(id), cancellationToken); - return result.Map(ToResponse).ToGetResult(); + return result.Map(vm => ToResponse(vm, entityLocker.IsPlayoutLocked(id))).ToGetResult(); } [HttpGet("/api/playouts/{id:int}/items", Name = "GetPlayoutItems")] @@ -128,7 +128,9 @@ public class PlayoutController(IMediator mediator, IEntityLocker entityLocker) : Option playout = await mediator.Send(new GetPlayoutById(created.PlayoutId), cancellationToken); return playout.Match( - Some: vm => (IActionResult)new CreatedResult($"/api/playouts/{vm.PlayoutId}", ToResponse(vm)), + Some: vm => (IActionResult)new CreatedResult( + $"/api/playouts/{vm.PlayoutId}", + ToResponse(vm, entityLocker.IsPlayoutLocked(vm.PlayoutId))), None: () => ApiResults.NotFoundProblem()); }); }); @@ -194,7 +196,8 @@ public class PlayoutController(IMediator mediator, IEntityLocker entityLocker) : return result.Match( Left: error => error.ToErrorResult(), - Right: playout => (IActionResult)new OkObjectResult(ToResponse(playout))); + Right: playout => (IActionResult)new OkObjectResult( + ToResponse(playout, entityLocker.IsPlayoutLocked(id)))); } private async Task> UpdateScheduleFile( @@ -260,7 +263,7 @@ public class PlayoutController(IMediator mediator, IEntityLocker entityLocker) : Option refreshed = await mediator.Send(new GetPlayoutById(id), cancellationToken); return refreshed.Match( - Some: vm => (IActionResult)new OkObjectResult(ToResponse(vm)), + Some: vm => (IActionResult)new OkObjectResult(ToResponse(vm, entityLocker.IsPlayoutLocked(id))), None: () => ApiResults.NotFoundProblem()); } @@ -550,14 +553,19 @@ public class PlayoutController(IMediator mediator, IEntityLocker entityLocker) : [Tags("Playouts")] [EndpointSummary("Reset all playouts")] [EndpointGroupName("general")] - [ProducesResponseType(StatusCodes.Status202Accepted)] + [ProducesResponseType(typeof(ResetAllPlayoutsResponseModel), StatusCodes.Status202Accepted)] // No 409 lock guard here (unlike the id-keyed mutations): ResetAllPlayoutsHandler already // skips any playout whose build lock is held, matching Blazor. It is a fire-and-forget - // bulk enqueue, so it always accepts. See docs/decisions.md 2026-07-10. + // bulk enqueue, so it always accepts — the 202 body reports which playouts were queued and + // which were skipped (locked, or an unsupported ExternalJson/None kind). See docs/decisions.md 2026-07-10. public async Task ResetAll(CancellationToken cancellationToken) { - await mediator.Send(new ResetAllPlayouts(), cancellationToken); - return Accepted(); + ResetAllPlayoutsResult result = await mediator.Send(new ResetAllPlayouts(), cancellationToken); + var body = new ResetAllPlayoutsResponseModel( + result.QueuedPlayoutIds, + result.SkippedLocked, + result.SkippedUnsupported); + return new AcceptedResult((string)null, body); } [HttpPost("/api/playouts/{id:int}/erase-items", Name = "ErasePlayoutItems")] @@ -728,7 +736,7 @@ public class PlayoutController(IMediator mediator, IEntityLocker entityLocker) : private static PlayoutHistoryDetailsResponseModel ToDetailsResponse(PlayoutHistoryDetailsViewModel vm) => new(vm.PlaybackOrder, vm.CollectionType, vm.Name, vm.MediaItemType, vm.MediaItemTitle); - private static PlayoutResponseModel ToResponse(PlayoutNameViewModel vm) => + private static PlayoutResponseModel ToResponse(PlayoutNameViewModel vm, bool isLocked) => PlayoutResponseModel.From( vm.PlayoutId, vm.ScheduleKind, @@ -740,7 +748,8 @@ public class PlayoutController(IMediator mediator, IEntityLocker entityLocker) : vm.DbDailyRebuildTime, ToBuildStatus(vm.BuildStatus), vm.DecoId, - vm.DecoName); + vm.DecoName, + isLocked); private static PlayoutAlternateScheduleResponseModel ToResponse(PlayoutAlternateScheduleViewModel vm) => new( diff --git a/docs/api-conventions.md b/docs/api-conventions.md index 38a944ce1..14861a905 100644 --- a/docs/api-conventions.md +++ b/docs/api-conventions.md @@ -112,12 +112,21 @@ Established by issue #215 (`PlayoutController` + `ChannelController.ResetPlayout `[ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)]` to each guarded action. Precedent for the 409 shape: `TraktController` (its private `ConflictProblem()`). Two nuances: - **Fire-and-forget bulk operations don't 409** — `POST /api/playouts/reset-all` stays 202; its - handler (`ResetAllPlayoutsHandler`) already *skips* locked playouts, matching Blazor + the handler - semantics. Only per-id mutations 409. + handler (`ResetAllPlayoutsHandler`) *skips* locked playouts, matching Blazor + the handler + semantics. Only per-id mutations 409. As of #235 the handler returns a `ResetAllPlayoutsResult` + (`QueuedPlayoutIds` / `SkippedLocked` / `SkippedUnsupported`) and the controller returns the 202 + **with a `ResetAllPlayoutsResponseModel` body** reporting what was queued vs. skipped (locked, or + an unsupported `ExternalJson`/`None` kind) — a fire-and-forget bulk op still reports its outcome + rather than silently swallowing skips. - **Surface the lock state to clients** so they can pre-disable the buttons: stamp an `IsLocked` boolean onto the list DTO (`PlayoutListItemResponseModel`, set from `IsPlayoutLocked` in the - controller's list projection) rather than adding a push channel. The SPA reads it and, on a 409, - refreshes the list to pick up the flag. + controller's list projection) rather than adding a push channel — and (as of #235) onto the + single-playout GET DTO (`PlayoutResponseModel.IsLocked`, set the same way in every action that + maps it) so a client polling one playout has the same flag. The SPA reads it and, on a 409, + refreshes to pick up the flag. +- **Async-op success is 202, not 200** — an endpoint whose success path only *queues* a background + rebuild returns **202 Accepted**, not 200 (#235: `POST /api/channels/{channelNumber}/playout/reset` + queues a `BuildPlayout` → `AcceptedResult`). Reserve 200 for a synchronous durable result. ### 3b. Map a "queue a background job" outcome to status codes with an enum, not a `bool` From 628c9d722818ecb0fd603d182549fcada4613356 Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 11 Jul 2026 18:02:17 +0200 Subject: [PATCH 06/12] =?UTF-8?q?feat(235):=20F9=20API=20parity=20?= =?UTF-8?q?=E2=80=94=20library=20deep-scan,=20external-collections=20scan,?= =?UTF-8?q?=20scan-show=20outcome=20enum=20(#235=20slice=20B)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the two F9 Libraries.razor parity gaps and normalizes scan-show error mapping to ProblemDetails. TASK 1 — library-wide deep scan: - QueueLibraryScanByLibraryId gains optional `bool DeepScan = false`; handler threads it into ForceSynchronize{Plex,Jellyfin,Emby}LibraryById. - POST /api/libraries/{id}/scan?deep=false binds it via [FromQuery]. TASK 2 — external-collections scan (new endpoints): - POST /api/media-sources/{plex|jellyfin|emby}/{id}/scan-collections?deep=false acquires the per-source collections lock (§3b: lock IS the running scan → 409), enqueues Synchronize{X}Collections(id, ForceScan:true, deep) to the scanner channel, returns 202; compensating-unlock on enqueue throw. TASK 3 — scan-show normalization: - New QueueShowScanResult enum; handler returns it instead of bool. - POST /api/libraries/{id}/scan-show now maps 202/404/409/422 (all errors ProblemDetails) instead of 200/404/400-anonymous-object. - Updated the lone Blazor caller (TelevisionSeasonList.razor). Tests: LibrariesController (scan deep=true, scan-show enum→status), the three media-source controllers (scan-collections route/404/409/202/compensating-unlock), and handler tests for both changed handlers (deep threading + show-scan outcomes). Docs: api-conventions §3b exemplar + blazor-route-parity §5 F9 gate. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Commands/QueueLibraryScanByLibraryId.cs | 2 +- .../QueueLibraryScanByLibraryIdHandler.cs | 6 +- .../Commands/QueueShowScanByLibraryId.cs | 13 +- .../QueueShowScanByLibraryIdHandler.cs | 26 ++- ...QueueLibraryScanByLibraryIdHandlerTests.cs | 32 ++- .../QueueShowScanByLibraryIdHandlerTests.cs | 214 ++++++++++++++++++ .../EmbyMediaSourcesControllerTests.cs | 46 ++++ .../JellyfinMediaSourcesControllerTests.cs | 46 ++++ .../Controllers/LibrariesControllerTests.cs | 91 +++++++- .../PlexMediaSourcesControllerTests.cs | 62 +++++ .../Api/EmbyMediaSourcesController.cs | 47 ++++ .../Api/JellyfinMediaSourcesController.cs | 47 ++++ .../Controllers/Api/LibrariesController.cs | 49 +++- .../Api/PlexMediaSourcesController.cs | 45 ++++ ErsatzTV/Pages/TelevisionSeasonList.razor | 5 +- docs/api-conventions.md | 9 + docs/blazor-route-parity.md | 6 +- 17 files changed, 704 insertions(+), 42 deletions(-) create mode 100644 ErsatzTV.Tests/Application/Libraries/QueueShowScanByLibraryIdHandlerTests.cs diff --git a/ErsatzTV.Application/Libraries/Commands/QueueLibraryScanByLibraryId.cs b/ErsatzTV.Application/Libraries/Commands/QueueLibraryScanByLibraryId.cs index 024b47be3..4b64fd73b 100644 --- a/ErsatzTV.Application/Libraries/Commands/QueueLibraryScanByLibraryId.cs +++ b/ErsatzTV.Application/Libraries/Commands/QueueLibraryScanByLibraryId.cs @@ -8,4 +8,4 @@ public enum QueueLibraryScanResult AlreadyScanning } -public record QueueLibraryScanByLibraryId(int LibraryId) : IRequest; +public record QueueLibraryScanByLibraryId(int LibraryId, bool DeepScan = false) : IRequest; diff --git a/ErsatzTV.Application/Libraries/Commands/QueueLibraryScanByLibraryIdHandler.cs b/ErsatzTV.Application/Libraries/Commands/QueueLibraryScanByLibraryIdHandler.cs index 6bb6a5f4b..8782e6631 100644 --- a/ErsatzTV.Application/Libraries/Commands/QueueLibraryScanByLibraryIdHandler.cs +++ b/ErsatzTV.Application/Libraries/Commands/QueueLibraryScanByLibraryIdHandler.cs @@ -66,7 +66,7 @@ public class QueueLibraryScanByLibraryIdHandler( new SynchronizePlexLibraries(library.MediaSourceId), cancellationToken); await scannerWorker.WriteAsync( - new ForceSynchronizePlexLibraryById(library.Id, false), + new ForceSynchronizePlexLibraryById(library.Id, request.DeepScan), cancellationToken); break; case JellyfinLibrary: @@ -74,7 +74,7 @@ public class QueueLibraryScanByLibraryIdHandler( new SynchronizeJellyfinLibraries(library.MediaSourceId), cancellationToken); await scannerWorker.WriteAsync( - new ForceSynchronizeJellyfinLibraryById(library.Id, false), + new ForceSynchronizeJellyfinLibraryById(library.Id, request.DeepScan), cancellationToken); break; case EmbyLibrary: @@ -82,7 +82,7 @@ public class QueueLibraryScanByLibraryIdHandler( new SynchronizeEmbyLibraries(library.MediaSourceId), cancellationToken); await scannerWorker.WriteAsync( - new ForceSynchronizeEmbyLibraryById(library.Id, false), + new ForceSynchronizeEmbyLibraryById(library.Id, request.DeepScan), cancellationToken); break; } diff --git a/ErsatzTV.Application/Libraries/Commands/QueueShowScanByLibraryId.cs b/ErsatzTV.Application/Libraries/Commands/QueueShowScanByLibraryId.cs index e4b1bf194..886a76d19 100644 --- a/ErsatzTV.Application/Libraries/Commands/QueueShowScanByLibraryId.cs +++ b/ErsatzTV.Application/Libraries/Commands/QueueShowScanByLibraryId.cs @@ -1,3 +1,14 @@ namespace ErsatzTV.Application.Libraries; -public record QueueShowScanByLibraryId(int LibraryId, int ShowId, string ShowTitle, bool DeepScan) : IRequest; +public enum QueueShowScanResult +{ + Queued, + NotFound, + Unsupported, + SyncDisabled, + AlreadyScanning, + ScanFailed +} + +public record QueueShowScanByLibraryId(int LibraryId, int ShowId, string ShowTitle, bool DeepScan) + : IRequest; diff --git a/ErsatzTV.Application/Libraries/Commands/QueueShowScanByLibraryIdHandler.cs b/ErsatzTV.Application/Libraries/Commands/QueueShowScanByLibraryIdHandler.cs index 281606797..5d3c93f3c 100644 --- a/ErsatzTV.Application/Libraries/Commands/QueueShowScanByLibraryIdHandler.cs +++ b/ErsatzTV.Application/Libraries/Commands/QueueShowScanByLibraryIdHandler.cs @@ -19,9 +19,9 @@ public class QueueShowScanByLibraryIdHandler( IMediator mediator, ChannelWriter workerChannel, ILogger logger) - : IRequestHandler + : IRequestHandler { - public async Task Handle(QueueShowScanByLibraryId request, CancellationToken cancellationToken) + public async Task Handle(QueueShowScanByLibraryId request, CancellationToken cancellationToken) { await using TvContext dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); @@ -42,14 +42,14 @@ public class QueueShowScanByLibraryIdHandler( if (!shouldSyncItems) { logger.LogWarning("Library sync is disabled for library id {Id}", library.Id); - return false; + return QueueShowScanResult.SyncDisabled; } - // Check if library is already being scanned - return false if locked + // A false from LockLibrary means a scan is already in progress; we own no release. if (!locker.LockLibrary(library.Id)) { logger.LogWarning("Library {Id} is already being scanned, cannot scan individual show", library.Id); - return false; + return QueueShowScanResult.AlreadyScanning; } logger.LogDebug( @@ -60,41 +60,43 @@ public class QueueShowScanByLibraryIdHandler( try { - var success = false; + QueueShowScanResult outcome; switch (library) { case PlexLibrary: Either plexResult = await mediator.Send( new SynchronizePlexShowById(library.Id, request.ShowId, request.DeepScan), cancellationToken); - success = plexResult.IsRight; + outcome = plexResult.IsRight ? QueueShowScanResult.Queued : QueueShowScanResult.ScanFailed; break; case JellyfinLibrary: Either jellyfinResult = await mediator.Send( new SynchronizeJellyfinShowById(library.Id, request.ShowId, request.DeepScan), cancellationToken); - success = jellyfinResult.IsRight; + outcome = jellyfinResult.IsRight ? QueueShowScanResult.Queued : QueueShowScanResult.ScanFailed; break; case EmbyLibrary: Either embyResult = await mediator.Send( new SynchronizeEmbyShowById(library.Id, request.ShowId, request.DeepScan), cancellationToken); - success = embyResult.IsRight; + outcome = embyResult.IsRight ? QueueShowScanResult.Queued : QueueShowScanResult.ScanFailed; break; case LocalLibrary: logger.LogWarning("Single show scanning is not supported for local libraries"); + outcome = QueueShowScanResult.Unsupported; break; default: logger.LogWarning("Unknown library type for library {Id}", library.Id); + outcome = QueueShowScanResult.Unsupported; break; } - if (success && request.DeepScan) + if (outcome == QueueShowScanResult.Queued && request.DeepScan) { await workerChannel.WriteAsync(new ExtractEmbeddedShowSubtitles(request.ShowId), cancellationToken); } - return success; + return outcome; } finally { @@ -103,6 +105,6 @@ public class QueueShowScanByLibraryIdHandler( } } - return false; + return QueueShowScanResult.NotFound; } } diff --git a/ErsatzTV.Tests/Application/Libraries/QueueLibraryScanByLibraryIdHandlerTests.cs b/ErsatzTV.Tests/Application/Libraries/QueueLibraryScanByLibraryIdHandlerTests.cs index c25cde87b..61fd248ef 100644 --- a/ErsatzTV.Tests/Application/Libraries/QueueLibraryScanByLibraryIdHandlerTests.cs +++ b/ErsatzTV.Tests/Application/Libraries/QueueLibraryScanByLibraryIdHandlerTests.cs @@ -164,6 +164,30 @@ public class QueueLibraryScanByLibraryIdHandlerTests locker.Received(1).UnlockLibrary(libraryId); } + [Test] + public async Task Handle_Should_Thread_DeepScan_Into_Plex_ForceSynchronize() + { + int libraryId = await SeedSyncEnabledPlexLibrary(); + + var locker = Substitute.For(); + locker.LockLibrary(libraryId).Returns(true); + Channel channel = ThreadingChannel.CreateUnbounded(); + + QueueLibraryScanByLibraryIdHandler handler = CreateHandler(locker, channel.Writer); + + QueueLibraryScanResult result = await handler.Handle( + new QueueLibraryScanByLibraryId(libraryId, DeepScan: true), + CancellationToken.None); + + result.ShouldBe(QueueLibraryScanResult.Queued); + + // first message refreshes the library list, second is the deep force-sync carrying DeepScan == true + channel.Reader.TryRead(out IScannerBackgroundServiceRequest? first).ShouldBeTrue(); + first.ShouldBeOfType(); + channel.Reader.TryRead(out IScannerBackgroundServiceRequest? second).ShouldBeTrue(); + second.ShouldBeOfType().DeepScan.ShouldBeTrue(); + } + private QueueLibraryScanByLibraryIdHandler CreateHandler( IEntityLocker locker, ChannelWriter writer) => @@ -193,7 +217,11 @@ public class QueueLibraryScanByLibraryIdHandlerTests return source.Libraries[0].Id; } - private async Task SeedSyncDisabledPlexLibrary() + private async Task SeedSyncDisabledPlexLibrary() => await SeedPlexLibrary(shouldSyncItems: false); + + private async Task SeedSyncEnabledPlexLibrary() => await SeedPlexLibrary(shouldSyncItems: true); + + private async Task SeedPlexLibrary(bool shouldSyncItems) { await using TvContext context = _db.CreateContext(); var source = new PlexMediaSource @@ -212,7 +240,7 @@ public class QueueLibraryScanByLibraryIdHandlerTests Name = "Plex Movies", MediaKind = LibraryMediaKind.Movies, Key = "1", - ShouldSyncItems = false, + ShouldSyncItems = shouldSyncItems, Paths = [] } ] diff --git a/ErsatzTV.Tests/Application/Libraries/QueueShowScanByLibraryIdHandlerTests.cs b/ErsatzTV.Tests/Application/Libraries/QueueShowScanByLibraryIdHandlerTests.cs new file mode 100644 index 000000000..e9aba9722 --- /dev/null +++ b/ErsatzTV.Tests/Application/Libraries/QueueShowScanByLibraryIdHandlerTests.cs @@ -0,0 +1,214 @@ +using System.Threading.Channels; +using ErsatzTV.Application; +using ErsatzTV.Application.Libraries; +using ErsatzTV.Application.Plex; +using ErsatzTV.Application.Subtitles; +using ErsatzTV.Core; +using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Interfaces.Locking; +using ErsatzTV.Infrastructure.Data; +using ErsatzTV.Tests.Support; +using LanguageExt; +using MediatR; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; +using NUnit.Framework; +using Shouldly; +using static LanguageExt.Prelude; +using ThreadingChannel = System.Threading.Channels.Channel; + +namespace ErsatzTV.Tests.Application.Libraries; + +[TestFixture] +public class QueueShowScanByLibraryIdHandlerTests +{ + private InMemoryTvContext _db = null!; + + [SetUp] + public async Task SetUp() => _db = await InMemoryTvContext.CreateAsync(); + + [TearDown] + public async Task TearDown() => await _db.DisposeAsync(); + + [Test] + public async Task Handle_Should_Return_NotFound_When_Library_Missing() + { + var locker = Substitute.For(); + var mediator = Substitute.For(); + + QueueShowScanByLibraryIdHandler handler = CreateHandler(locker, mediator, out _); + + QueueShowScanResult result = await handler.Handle( + new QueueShowScanByLibraryId(9999, 1, "Show", false), + CancellationToken.None); + + result.ShouldBe(QueueShowScanResult.NotFound); + locker.DidNotReceive().LockLibrary(Arg.Any()); + } + + [Test] + public async Task Handle_Should_Return_SyncDisabled_When_Item_Sync_Off() + { + int libraryId = await SeedPlexLibrary(shouldSyncItems: false); + var locker = Substitute.For(); + var mediator = Substitute.For(); + + QueueShowScanByLibraryIdHandler handler = CreateHandler(locker, mediator, out _); + + QueueShowScanResult result = await handler.Handle( + new QueueShowScanByLibraryId(libraryId, 1, "Show", false), + CancellationToken.None); + + result.ShouldBe(QueueShowScanResult.SyncDisabled); + locker.DidNotReceive().LockLibrary(Arg.Any()); + } + + [Test] + public async Task Handle_Should_Return_AlreadyScanning_When_Lock_Not_Acquired() + { + int libraryId = await SeedPlexLibrary(shouldSyncItems: true); + var locker = Substitute.For(); + locker.LockLibrary(libraryId).Returns(false); + var mediator = Substitute.For(); + + QueueShowScanByLibraryIdHandler handler = CreateHandler(locker, mediator, out _); + + QueueShowScanResult result = await handler.Handle( + new QueueShowScanByLibraryId(libraryId, 1, "Show", false), + CancellationToken.None); + + result.ShouldBe(QueueShowScanResult.AlreadyScanning); + await mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + } + + [Test] + public async Task Handle_Should_Return_Unsupported_For_Local_Library() + { + int libraryId = await SeedLocalLibrary(); + var locker = Substitute.For(); + locker.LockLibrary(libraryId).Returns(true); + var mediator = Substitute.For(); + + QueueShowScanByLibraryIdHandler handler = CreateHandler(locker, mediator, out _); + + QueueShowScanResult result = await handler.Handle( + new QueueShowScanByLibraryId(libraryId, 1, "Show", false), + CancellationToken.None); + + result.ShouldBe(QueueShowScanResult.Unsupported); + locker.Received(1).UnlockLibrary(libraryId); + } + + [Test] + public async Task Handle_Should_Return_Queued_And_Extract_Subtitles_On_Deep_Success() + { + int libraryId = await SeedPlexLibrary(shouldSyncItems: true); + var locker = Substitute.For(); + locker.LockLibrary(libraryId).Returns(true); + var mediator = Substitute.For(); + mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Right("ok")); + + QueueShowScanByLibraryIdHandler handler = CreateHandler( + locker, + mediator, + out Channel worker); + + QueueShowScanResult result = await handler.Handle( + new QueueShowScanByLibraryId(libraryId, 42, "Show", DeepScan: true), + CancellationToken.None); + + result.ShouldBe(QueueShowScanResult.Queued); + locker.Received(1).UnlockLibrary(libraryId); + worker.Reader.TryRead(out IBackgroundServiceRequest? request).ShouldBeTrue(); + request.ShouldBeOfType().ShowId.ShouldBe(42); + } + + [Test] + public async Task Handle_Should_Return_ScanFailed_When_SubScan_Left() + { + int libraryId = await SeedPlexLibrary(shouldSyncItems: true); + var locker = Substitute.For(); + locker.LockLibrary(libraryId).Returns(true); + var mediator = Substitute.For(); + mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Left(BaseError.New("scan error"))); + + QueueShowScanByLibraryIdHandler handler = CreateHandler( + locker, + mediator, + out Channel worker); + + QueueShowScanResult result = await handler.Handle( + new QueueShowScanByLibraryId(libraryId, 42, "Show", DeepScan: true), + CancellationToken.None); + + result.ShouldBe(QueueShowScanResult.ScanFailed); + locker.Received(1).UnlockLibrary(libraryId); + // no subtitle extraction on a failed scan + worker.Reader.TryRead(out _).ShouldBeFalse(); + } + + private QueueShowScanByLibraryIdHandler CreateHandler( + IEntityLocker locker, + IMediator mediator, + out Channel worker) + { + worker = ThreadingChannel.CreateUnbounded(); + return new QueueShowScanByLibraryIdHandler( + _db.Factory, + locker, + mediator, + worker.Writer, + NullLogger.Instance); + } + + private async Task SeedLocalLibrary() + { + await using TvContext context = _db.CreateContext(); + var source = new LocalMediaSource + { + Libraries = + [ + new LocalLibrary + { + Name = "Local Movies", + MediaKind = LibraryMediaKind.Movies, + Paths = [] + } + ] + }; + await context.LocalMediaSources.AddAsync(source); + await context.SaveChangesAsync(); + return source.Libraries[0].Id; + } + + private async Task SeedPlexLibrary(bool shouldSyncItems) + { + await using TvContext context = _db.CreateContext(); + var source = new PlexMediaSource + { + ServerName = "Plex Server", + ProductVersion = "1", + Platform = "Linux", + PlatformVersion = "1", + ClientIdentifier = "plex", + Connections = [], + PathReplacements = [], + Libraries = + [ + new PlexLibrary + { + Name = "Plex Shows", + MediaKind = LibraryMediaKind.Shows, + Key = "1", + ShouldSyncItems = shouldSyncItems, + Paths = [] + } + ] + }; + await context.PlexMediaSources.AddAsync(source); + await context.SaveChangesAsync(); + return source.Libraries[0].Id; + } +} diff --git a/ErsatzTV.Tests/Controllers/EmbyMediaSourcesControllerTests.cs b/ErsatzTV.Tests/Controllers/EmbyMediaSourcesControllerTests.cs index d5a672677..cd838dcd8 100644 --- a/ErsatzTV.Tests/Controllers/EmbyMediaSourcesControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/EmbyMediaSourcesControllerTests.cs @@ -75,6 +75,10 @@ public class EmbyMediaSourcesControllerTests nameof(EmbyMediaSourcesController.RefreshLibraries), "POST", "/api/media-sources/emby/{id:int}/refresh-libraries"); + ShouldHaveActionRoute( + nameof(EmbyMediaSourcesController.ScanCollections), + "POST", + "/api/media-sources/emby/{id:int}/scan-collections"); } [Test] @@ -414,6 +418,48 @@ public class EmbyMediaSourcesControllerTests request.ShouldBeOfType().EmbyMediaSourceId.ShouldBe(1); } + [Test] + public async Task ScanCollections_Should_Return_404_When_Source_Missing() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.None); + + IActionResult result = await _controller.ScanCollections(99, cancellationToken: CancellationToken.None); + + result.ShouldBeOfType(); + _entityLocker.DidNotReceive().LockEmbyCollections(); + } + + [Test] + public async Task ScanCollections_Should_Return_409_When_Collections_Locked() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(new EmbyMediaSourceViewModel(1, "s", "a"))); + _entityLocker.LockEmbyCollections().Returns(false); + + IActionResult result = await _controller.ScanCollections(1, cancellationToken: CancellationToken.None); + + result.ShouldBeOfType().Value.ShouldBeOfType().Status.ShouldBe(409); + _scannerChannel.Reader.TryRead(out _).ShouldBeFalse(); + } + + [Test] + public async Task ScanCollections_Should_Enqueue_And_Return_202() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(new EmbyMediaSourceViewModel(1, "s", "a"))); + _entityLocker.LockEmbyCollections().Returns(true); + + IActionResult result = await _controller.ScanCollections(1, deep: true, CancellationToken.None); + + result.ShouldBeOfType(); + _scannerChannel.Reader.TryRead(out IScannerBackgroundServiceRequest? request).ShouldBeTrue(); + var command = request.ShouldBeOfType(); + command.EmbyMediaSourceId.ShouldBe(1); + command.ForceScan.ShouldBeTrue(); + command.DeepScan.ShouldBeTrue(); + } + private static void ShouldHaveActionRoute(string actionName, string httpMethod, string route) { MethodInfo action = typeof(EmbyMediaSourcesController).GetMethod(actionName) diff --git a/ErsatzTV.Tests/Controllers/JellyfinMediaSourcesControllerTests.cs b/ErsatzTV.Tests/Controllers/JellyfinMediaSourcesControllerTests.cs index 5623b5dfa..a891570bf 100644 --- a/ErsatzTV.Tests/Controllers/JellyfinMediaSourcesControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/JellyfinMediaSourcesControllerTests.cs @@ -75,6 +75,10 @@ public class JellyfinMediaSourcesControllerTests nameof(JellyfinMediaSourcesController.RefreshLibraries), "POST", "/api/media-sources/jellyfin/{id:int}/refresh-libraries"); + ShouldHaveActionRoute( + nameof(JellyfinMediaSourcesController.ScanCollections), + "POST", + "/api/media-sources/jellyfin/{id:int}/scan-collections"); } [Test] @@ -414,6 +418,48 @@ public class JellyfinMediaSourcesControllerTests request.ShouldBeOfType().JellyfinMediaSourceId.ShouldBe(1); } + [Test] + public async Task ScanCollections_Should_Return_404_When_Source_Missing() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.None); + + IActionResult result = await _controller.ScanCollections(99, cancellationToken: CancellationToken.None); + + result.ShouldBeOfType(); + _entityLocker.DidNotReceive().LockJellyfinCollections(); + } + + [Test] + public async Task ScanCollections_Should_Return_409_When_Collections_Locked() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(new JellyfinMediaSourceViewModel(1, "s", "a"))); + _entityLocker.LockJellyfinCollections().Returns(false); + + IActionResult result = await _controller.ScanCollections(1, cancellationToken: CancellationToken.None); + + result.ShouldBeOfType().Value.ShouldBeOfType().Status.ShouldBe(409); + _scannerChannel.Reader.TryRead(out _).ShouldBeFalse(); + } + + [Test] + public async Task ScanCollections_Should_Enqueue_And_Return_202() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(new JellyfinMediaSourceViewModel(1, "s", "a"))); + _entityLocker.LockJellyfinCollections().Returns(true); + + IActionResult result = await _controller.ScanCollections(1, deep: true, CancellationToken.None); + + result.ShouldBeOfType(); + _scannerChannel.Reader.TryRead(out IScannerBackgroundServiceRequest? request).ShouldBeTrue(); + var command = request.ShouldBeOfType(); + command.JellyfinMediaSourceId.ShouldBe(1); + command.ForceScan.ShouldBeTrue(); + command.DeepScan.ShouldBeTrue(); + } + private static void ShouldHaveActionRoute(string actionName, string httpMethod, string route) { MethodInfo action = typeof(JellyfinMediaSourcesController).GetMethod(actionName) diff --git a/ErsatzTV.Tests/Controllers/LibrariesControllerTests.cs b/ErsatzTV.Tests/Controllers/LibrariesControllerTests.cs index 95ebbdf6b..476758752 100644 --- a/ErsatzTV.Tests/Controllers/LibrariesControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/LibrariesControllerTests.cs @@ -71,14 +71,15 @@ public class LibrariesControllerTests } [Test] - public async Task ScanShow_Should_Queue_Scan_By_Show_Id_When_Show_Belongs_To_Library() + public async Task ScanShow_Should_Return_202_And_Queue_Scan_By_Show_Id_When_Show_Belongs_To_Library() { _televisionRepository.GetShowTitle(3, 42).Returns(Option.Some("The Office")); - _mediator.Send(Arg.Any(), Arg.Any()).Returns(true); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(QueueShowScanResult.Queued); IActionResult result = await _controller.ScanShow(3, new ScanShowRequest(42, DeepScan: true)); - result.ShouldBeOfType(); + result.ShouldBeOfType(); await _mediator.Received(1).Send( Arg.Is(r => r.LibraryId == 3 && r.ShowId == 42 && r.ShowTitle == "The Office" && r.DeepScan), @@ -86,14 +87,68 @@ public class LibrariesControllerTests } [Test] - public async Task ScanShow_Should_Return_BadRequest_When_Mediator_Fails_To_Queue() + public async Task ScanShow_Should_Return_409_When_AlreadyScanning() { _televisionRepository.GetShowTitle(3, 42).Returns(Option.Some("The Office")); - _mediator.Send(Arg.Any(), Arg.Any()).Returns(false); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(QueueShowScanResult.AlreadyScanning); IActionResult result = await _controller.ScanShow(3, new ScanShowRequest(42)); - result.ShouldBeOfType(); + var conflict = result.ShouldBeOfType(); + conflict.Value.ShouldBeOfType().Status.ShouldBe(StatusCodes.Status409Conflict); + } + + [Test] + public async Task ScanShow_Should_Return_422_When_SyncDisabled() + { + _televisionRepository.GetShowTitle(3, 42).Returns(Option.Some("The Office")); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(QueueShowScanResult.SyncDisabled); + + IActionResult result = await _controller.ScanShow(3, new ScanShowRequest(42)); + + var unprocessable = result.ShouldBeOfType(); + unprocessable.Value.ShouldBeOfType().Status.ShouldBe(StatusCodes.Status422UnprocessableEntity); + } + + [Test] + public async Task ScanShow_Should_Return_422_When_Unsupported() + { + _televisionRepository.GetShowTitle(3, 42).Returns(Option.Some("The Office")); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(QueueShowScanResult.Unsupported); + + IActionResult result = await _controller.ScanShow(3, new ScanShowRequest(42)); + + var unprocessable = result.ShouldBeOfType(); + unprocessable.Value.ShouldBeOfType().Status.ShouldBe(StatusCodes.Status422UnprocessableEntity); + } + + [Test] + public async Task ScanShow_Should_Return_422_When_ScanFailed() + { + _televisionRepository.GetShowTitle(3, 42).Returns(Option.Some("The Office")); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(QueueShowScanResult.ScanFailed); + + IActionResult result = await _controller.ScanShow(3, new ScanShowRequest(42)); + + var unprocessable = result.ShouldBeOfType(); + unprocessable.Value.ShouldBeOfType().Status.ShouldBe(StatusCodes.Status422UnprocessableEntity); + } + + [Test] + public async Task ScanShow_Should_Return_404_When_Handler_Reports_NotFound() + { + _televisionRepository.GetShowTitle(3, 42).Returns(Option.Some("The Office")); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(QueueShowScanResult.NotFound); + + IActionResult result = await _controller.ScanShow(3, new ScanShowRequest(42)); + + var notFound = result.ShouldBeOfType(); + notFound.Value.ShouldBeOfType().Status.ShouldBe(StatusCodes.Status404NotFound); } [Test] @@ -102,11 +157,25 @@ public class LibrariesControllerTests _mediator.Send(Arg.Any(), Arg.Any()) .Returns(QueueLibraryScanResult.Queued); - IActionResult result = await _controller.ScanLibrary(7, CancellationToken.None); + IActionResult result = await _controller.ScanLibrary(7, cancellationToken: CancellationToken.None); result.ShouldBeOfType(); await _mediator.Received(1).Send( - Arg.Is(r => r.LibraryId == 7), + Arg.Is(r => r.LibraryId == 7 && !r.DeepScan), + Arg.Any()); + } + + [Test] + public async Task ScanLibrary_Should_Pass_DeepScan_When_Deep_Query_True() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(QueueLibraryScanResult.Queued); + + IActionResult result = await _controller.ScanLibrary(7, deep: true, CancellationToken.None); + + result.ShouldBeOfType(); + await _mediator.Received(1).Send( + Arg.Is(r => r.LibraryId == 7 && r.DeepScan), Arg.Any()); } @@ -116,7 +185,7 @@ public class LibrariesControllerTests _mediator.Send(Arg.Any(), Arg.Any()) .Returns(QueueLibraryScanResult.NotFound); - IActionResult result = await _controller.ScanLibrary(7, CancellationToken.None); + IActionResult result = await _controller.ScanLibrary(7, cancellationToken: CancellationToken.None); var notFound = result.ShouldBeOfType(); notFound.Value.ShouldBeOfType().Status.ShouldBe(StatusCodes.Status404NotFound); @@ -128,7 +197,7 @@ public class LibrariesControllerTests _mediator.Send(Arg.Any(), Arg.Any()) .Returns(QueueLibraryScanResult.AlreadyScanning); - IActionResult result = await _controller.ScanLibrary(7, CancellationToken.None); + IActionResult result = await _controller.ScanLibrary(7, cancellationToken: CancellationToken.None); var conflict = result.ShouldBeOfType(); conflict.Value.ShouldBeOfType().Status.ShouldBe(StatusCodes.Status409Conflict); @@ -140,7 +209,7 @@ public class LibrariesControllerTests _mediator.Send(Arg.Any(), Arg.Any()) .Returns(QueueLibraryScanResult.SyncDisabled); - IActionResult result = await _controller.ScanLibrary(7, CancellationToken.None); + IActionResult result = await _controller.ScanLibrary(7, cancellationToken: CancellationToken.None); var unprocessable = result.ShouldBeOfType(); unprocessable.Value.ShouldBeOfType().Status.ShouldBe(StatusCodes.Status422UnprocessableEntity); diff --git a/ErsatzTV.Tests/Controllers/PlexMediaSourcesControllerTests.cs b/ErsatzTV.Tests/Controllers/PlexMediaSourcesControllerTests.cs index 4c66dacd1..a2a9a4ec2 100644 --- a/ErsatzTV.Tests/Controllers/PlexMediaSourcesControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/PlexMediaSourcesControllerTests.cs @@ -70,6 +70,10 @@ public class PlexMediaSourcesControllerTests nameof(PlexMediaSourcesController.RefreshLibraries), "POST", "/api/media-sources/plex/{id:int}/refresh-libraries"); + ShouldHaveActionRoute( + nameof(PlexMediaSourcesController.ScanCollections), + "POST", + "/api/media-sources/plex/{id:int}/scan-collections"); } // ----- P1 GetState ----- @@ -452,6 +456,64 @@ public class PlexMediaSourcesControllerTests Arg.Any()); } + // ----- P9 ScanCollections ----- + + [Test] + public async Task ScanCollections_Should_Return_404_When_Source_Missing() + { + SourceExists(false); + + IActionResult result = await _controller.ScanCollections(9, cancellationToken: CancellationToken.None); + + result.ShouldBeOfType(); + _entityLocker.DidNotReceive().LockPlexCollections(); + await _channel.DidNotReceive().WriteAsync( + Arg.Any(), + Arg.Any()); + } + + [Test] + public async Task ScanCollections_Should_Return_409_When_Collections_Locked() + { + SourceExists(true); + _entityLocker.LockPlexCollections().Returns(false); + + IActionResult result = await _controller.ScanCollections(3, cancellationToken: CancellationToken.None); + + result.ShouldBeOfType().Value.ShouldBeOfType().Status.ShouldBe(409); + await _channel.DidNotReceive().WriteAsync( + Arg.Any(), + Arg.Any()); + } + + [Test] + public async Task ScanCollections_Should_Return_202_And_Enqueue() + { + SourceExists(true); + _entityLocker.LockPlexCollections().Returns(true); + + IActionResult result = await _controller.ScanCollections(3, deep: true, CancellationToken.None); + + result.ShouldBeOfType(); + await _channel.Received(1).WriteAsync( + Arg.Is(s => s.PlexMediaSourceId == 3 && s.ForceScan && s.DeepScan), + Arg.Any()); + } + + [Test] + public async Task ScanCollections_Should_Compensate_Unlock_When_Enqueue_Throws() + { + SourceExists(true); + _entityLocker.LockPlexCollections().Returns(true); + _channel.WriteAsync(Arg.Any(), Arg.Any()) + .Returns(_ => throw new InvalidOperationException("channel closed")); + + await Should.ThrowAsync( + () => _controller.ScanCollections(3, cancellationToken: CancellationToken.None)); + + _entityLocker.Received(1).UnlockPlexCollections(); + } + private void SourceExists(bool exists) => _mediator.Send(Arg.Any(), Arg.Any()) .Returns(exists diff --git a/ErsatzTV/Controllers/Api/EmbyMediaSourcesController.cs b/ErsatzTV/Controllers/Api/EmbyMediaSourcesController.cs index 9780e20f4..a63450fda 100644 --- a/ErsatzTV/Controllers/Api/EmbyMediaSourcesController.cs +++ b/ErsatzTV/Controllers/Api/EmbyMediaSourcesController.cs @@ -278,6 +278,53 @@ public class EmbyMediaSourcesController( return new AcceptedResult(); } + [HttpPost("/api/media-sources/emby/{id:int}/scan-collections", Name = "ScanEmbyCollections")] + [Tags("Emby")] + [EndpointSummary("Scan an Emby source's collections")] + [EndpointDescription( + "Queues a synchronization of the source's collections (fire-and-forget). Pass ?deep=true for a deep " + + "scan. Returns 409 while an Emby collections scan is already in progress.")] + [EndpointGroupName("general")] + [ProducesResponseType(StatusCodes.Status202Accepted)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] + public async Task ScanCollections( + int id, + [FromQuery] bool deep = false, + CancellationToken cancellationToken = default) + { + Option maybeSource = + await mediator.Send(new GetEmbyMediaSourceById(id), cancellationToken); + if (maybeSource.IsNone) + { + return ApiResults.NotFoundProblem(); + } + + // The collections lock IS the running scan (§3b): fail to acquire = a scan is already active → 409. + if (!entityLocker.LockEmbyCollections()) + { + return ApiResults.ConflictProblem( + "Emby collections scan in progress", + "An Emby collections scan is already in progress; try again once it completes."); + } + + try + { + await scannerWorkerChannel.WriteAsync( + new SynchronizeEmbyCollections(id, true, deep), + cancellationToken); + } + catch + { + // the scanner releases the lock when it processes the message; if the enqueue throws after we + // acquired the lock, release it here (EnqueueWithTraktLock compensating-unlock, §3b) + entityLocker.UnlockEmbyCollections(); + throw; + } + + return new AcceptedResult(); + } + // §C7: LockLibrary then enqueue the sync pair; a throw from the enqueue compensates by unlocking // (EnqueueWithTraktLock pattern) — a locked library is silently skipped (Blazor parity). private async Task EnqueueLibrarySync(int sourceId, int libraryId, CancellationToken cancellationToken) diff --git a/ErsatzTV/Controllers/Api/JellyfinMediaSourcesController.cs b/ErsatzTV/Controllers/Api/JellyfinMediaSourcesController.cs index aa767ed14..d3a96d296 100644 --- a/ErsatzTV/Controllers/Api/JellyfinMediaSourcesController.cs +++ b/ErsatzTV/Controllers/Api/JellyfinMediaSourcesController.cs @@ -278,6 +278,53 @@ public class JellyfinMediaSourcesController( return new AcceptedResult(); } + [HttpPost("/api/media-sources/jellyfin/{id:int}/scan-collections", Name = "ScanJellyfinCollections")] + [Tags("Jellyfin")] + [EndpointSummary("Scan a Jellyfin source's collections")] + [EndpointDescription( + "Queues a synchronization of the source's collections (fire-and-forget). Pass ?deep=true for a deep " + + "scan. Returns 409 while a Jellyfin collections scan is already in progress.")] + [EndpointGroupName("general")] + [ProducesResponseType(StatusCodes.Status202Accepted)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] + public async Task ScanCollections( + int id, + [FromQuery] bool deep = false, + CancellationToken cancellationToken = default) + { + Option maybeSource = + await mediator.Send(new GetJellyfinMediaSourceById(id), cancellationToken); + if (maybeSource.IsNone) + { + return ApiResults.NotFoundProblem(); + } + + // The collections lock IS the running scan (§3b): fail to acquire = a scan is already active → 409. + if (!entityLocker.LockJellyfinCollections()) + { + return ApiResults.ConflictProblem( + "Jellyfin collections scan in progress", + "A Jellyfin collections scan is already in progress; try again once it completes."); + } + + try + { + await scannerWorkerChannel.WriteAsync( + new SynchronizeJellyfinCollections(id, true, deep), + cancellationToken); + } + catch + { + // the scanner releases the lock when it processes the message; if the enqueue throws after we + // acquired the lock, release it here (EnqueueWithTraktLock compensating-unlock, §3b) + entityLocker.UnlockJellyfinCollections(); + throw; + } + + return new AcceptedResult(); + } + // §C7: LockLibrary then enqueue the sync pair; a throw from the enqueue compensates by unlocking // (EnqueueWithTraktLock pattern) — a locked library is silently skipped (Blazor parity). private async Task EnqueueLibrarySync(int sourceId, int libraryId, CancellationToken cancellationToken) diff --git a/ErsatzTV/Controllers/Api/LibrariesController.cs b/ErsatzTV/Controllers/Api/LibrariesController.cs index d8525a52d..d75a11a5e 100644 --- a/ErsatzTV/Controllers/Api/LibrariesController.cs +++ b/ErsatzTV/Controllers/Api/LibrariesController.cs @@ -22,13 +22,18 @@ public class LibrariesController(ITelevisionRepository televisionRepository, IMe [HttpPost("/api/libraries/{id:int}/scan")] [Tags("Libraries")] [EndpointSummary("Scan library")] + [EndpointDescription("Queues a scan of the whole library. Pass ?deep=true for a deep (metadata-refresh) scan.")] [ProducesResponseType(StatusCodes.Status202Accepted)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] - public async Task ScanLibrary(int id, CancellationToken cancellationToken) + public async Task ScanLibrary( + int id, + [FromQuery] bool deep = false, + CancellationToken cancellationToken = default) { - QueueLibraryScanResult result = await mediator.Send(new QueueLibraryScanByLibraryId(id), cancellationToken); + QueueLibraryScanResult result = + await mediator.Send(new QueueLibraryScanByLibraryId(id, deep), cancellationToken); return result switch { QueueLibraryScanResult.Queued => new AcceptedResult(), @@ -49,19 +54,47 @@ public class LibrariesController(ITelevisionRepository televisionRepository, IMe [HttpPost("/api/libraries/{id:int}/scan-show")] [Tags("Libraries")] [EndpointSummary("Scan show")] - [ProducesResponseType(StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status202Accepted)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] - [ProducesResponseType(StatusCodes.Status400BadRequest)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task ScanShow(int id, [FromBody] ScanShowRequest request) { Option maybeTitle = await televisionRepository.GetShowTitle(id, request.ShowId); foreach (string title in maybeTitle) { - bool result = await mediator.Send(new QueueShowScanByLibraryId(id, request.ShowId, title, request.DeepScan)); + QueueShowScanResult result = + await mediator.Send(new QueueShowScanByLibraryId(id, request.ShowId, title, request.DeepScan)); - return result - ? new OkResult() - : new BadRequestObjectResult(new { error = "Unable to queue show scan. Library may not exist, may not support single show scanning, or may already be scanning." }); + return result switch + { + QueueShowScanResult.Queued => new AcceptedResult(), + QueueShowScanResult.AlreadyScanning => ApiResults.ConflictProblem( + "Library scan in progress", + $"A scan for library {id} is already in progress; cannot scan an individual show."), + QueueShowScanResult.SyncDisabled => new UnprocessableEntityObjectResult( + new ProblemDetails + { + Status = StatusCodes.Status422UnprocessableEntity, + Title = "Library sync is disabled", + Detail = $"Item sync is disabled for library {id}." + }), + QueueShowScanResult.Unsupported => new UnprocessableEntityObjectResult( + new ProblemDetails + { + Status = StatusCodes.Status422UnprocessableEntity, + Title = "Single show scanning is not supported", + Detail = $"Library {id} does not support scanning an individual show." + }), + QueueShowScanResult.ScanFailed => new UnprocessableEntityObjectResult( + new ProblemDetails + { + Status = StatusCodes.Status422UnprocessableEntity, + Title = "Unable to scan show", + Detail = $"The scan for show {request.ShowId} in library {id} could not be completed." + }), + _ => ApiResults.NotFoundProblem($"Library {id} does not exist.") + }; } return ApiResults.NotFoundProblem($"Show {request.ShowId} does not exist in library {id}."); diff --git a/ErsatzTV/Controllers/Api/PlexMediaSourcesController.cs b/ErsatzTV/Controllers/Api/PlexMediaSourcesController.cs index e1cc451d6..ff6a48098 100644 --- a/ErsatzTV/Controllers/Api/PlexMediaSourcesController.cs +++ b/ErsatzTV/Controllers/Api/PlexMediaSourcesController.cs @@ -277,6 +277,51 @@ public class PlexMediaSourcesController( return new AcceptedResult(); } + [HttpPost("/api/media-sources/plex/{id:int}/scan-collections", Name = "ScanPlexCollections")] + [Tags("Plex")] + [EndpointSummary("Scan a Plex server's collections")] + [EndpointDescription( + "Queues a synchronization of the server's collections (fire-and-forget). Pass ?deep=true for a deep " + + "scan. Returns 409 while a Plex collections scan is already in progress.")] + [EndpointGroupName("general")] + [ProducesResponseType(StatusCodes.Status202Accepted)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] + public async Task ScanCollections( + int id, + [FromQuery] bool deep = false, + CancellationToken cancellationToken = default) + { + if (!await PlexSourceExists(id, cancellationToken)) + { + return ApiResults.NotFoundProblem(); + } + + // The collections lock IS the running scan (§3b): fail to acquire = a scan is already active → 409. + if (!entityLocker.LockPlexCollections()) + { + return ApiResults.ConflictProblem( + "Plex collections scan in progress", + "A Plex collections scan is already in progress; try again once it completes."); + } + + try + { + await scannerWorkerChannel.WriteAsync( + new SynchronizePlexCollections(id, true, deep), + cancellationToken); + } + catch + { + // the scanner releases the lock when it processes the message; if the enqueue throws after we + // acquired the lock, release it here (EnqueueWithTraktLock compensating-unlock, §3b) + entityLocker.UnlockPlexCollections(); + throw; + } + + return new AcceptedResult(); + } + private async Task PlexSourceExists(int id, CancellationToken cancellationToken) => (await mediator.Send(new GetPlexMediaSourceById(id), cancellationToken)).IsSome; diff --git a/ErsatzTV/Pages/TelevisionSeasonList.razor b/ErsatzTV/Pages/TelevisionSeasonList.razor index 40016caf6..af9e0f4c7 100644 --- a/ErsatzTV/Pages/TelevisionSeasonList.razor +++ b/ErsatzTV/Pages/TelevisionSeasonList.razor @@ -307,8 +307,9 @@ private async Task ScanShow(bool deepScan) { - bool result = await Mediator.Send(new QueueShowScanByLibraryId(_show.LibraryId, _show.Id, _show.Title, deepScan)); - if (!result) + QueueShowScanResult result = + await Mediator.Send(new QueueShowScanByLibraryId(_show.LibraryId, _show.Id, _show.Title, deepScan)); + if (result != QueueShowScanResult.Queued) { Snackbar.Add($"Unable to scan show {_show.Title}", Severity.Error); } diff --git a/docs/api-conventions.md b/docs/api-conventions.md index 38a944ce1..639868c98 100644 --- a/docs/api-conventions.md +++ b/docs/api-conventions.md @@ -135,6 +135,15 @@ operation itself, not a mutation racing it). Add `[ProducesResponseType]` for 20 `EnqueueWithTraktLock` compensating-unlock pattern (`TraktController`): if a `WriteAsync` throws after a successful `Lock*`, `Unlock*` in a `catch` and rethrow — one lock ⇄ exactly one release. +A second exemplar (issue #235 slice B), where the lock lives on the **controller** rather than in a +handler: `POST /api/media-sources/{plex|jellyfin|emby}/{id}/scan-collections?deep=` acquires the +per-source collections lock (`entityLocker.LockPlexCollections()` etc.) — the lock IS the running +collections scan, so a `false` = 409 — then `WriteAsync`es `Synchronize{X}Collections(id, ForceScan: +true, deep)` to the scanner channel and returns **202**. `ScannerService` releases that lock in a +`finally` when it processes the message; the controller compensating-unlocks in a `catch` if the +enqueue throws. `POST /api/libraries/{id}/scan?deep=` similarly threads an optional `[FromQuery] bool +deep` into `QueueLibraryScanByLibraryId(id, DeepScan)`. + `NotFoundError : BaseError` lives in `ErsatzTV.Core/Errors/NotFoundError.cs` — return it from a handler's validation when a lookup fails, so the controller-side mapping falls out for free. diff --git a/docs/blazor-route-parity.md b/docs/blazor-route-parity.md index 6ae11a304..1addc1409 100644 --- a/docs/blazor-route-parity.md +++ b/docs/blazor-route-parity.md @@ -270,8 +270,10 @@ intentional exit ramp until phase (b) removes Blazor entirely. ## Section 5 — Removal execution runbook (#91 phase b, Step 2/3) The removal PR is **gated** — it starts only after these clear: ~~#202 (media-source write API + SPA)~~ -**DONE 2026-07-11**, #235 F9 (deep-scan + external-collections-scan API — no API today, so `Libraries.razor` -can't be deleted yet), and the **mandatory cold adversarial pass** (#91 comment 2026-07-09). (#204's id-carrying pattern +**DONE 2026-07-11**, ~~#235 F9 (deep-scan + external-collections-scan API)~~ **API DONE (#235 slice B)**: +`POST /api/libraries/{id}/scan?deep=` now threads deep-scan, and `POST /api/media-sources/{plex|jellyfin|emby}/{id}/scan-collections?deep=` +covers external-collections scan (the two `Libraries.razor` parity gaps) — the SPA `Libraries.razor` port can now +proceed, and the **mandatory cold adversarial pass** (#91 comment 2026-07-09). (#204's id-carrying pattern redirects landed 2026-07-11; its catch-all fallback that replaces `MapFallbackToPage("/_Host")` is folded into Step 2 below, since it can only ship when `_Host` is deleted.) When those close, the removal-PR routine executes, in order: From a27cfc475e0453dd75fb2cb0710ffd33630c5f54 Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 11 Jul 2026 18:04:33 +0200 Subject: [PATCH 07/12] chore(235): regenerate OpenAPI artifacts + endpoint index after slice merge Co-Authored-By: Claude Opus 4.8 (1M context) --- ErsatzTV/wwwroot/openapi/v1.json | 423 ++++++++++++++++++++++++++++++- docs/endpoint-index.md | 5 +- web/src/api/generated/v1.d.ts | 6 + 3 files changed, 423 insertions(+), 11 deletions(-) diff --git a/ErsatzTV/wwwroot/openapi/v1.json b/ErsatzTV/wwwroot/openapi/v1.json index 21666f098..01ed64660 100644 --- a/ErsatzTV/wwwroot/openapi/v1.json +++ b/ErsatzTV/wwwroot/openapi/v1.json @@ -1822,8 +1822,8 @@ } ], "responses": { - "200": { - "description": "OK" + "202": { + "description": "Accepted" }, "404": { "description": "Not Found", @@ -4798,6 +4798,80 @@ } } }, + "/api/media-sources/emby/{id}/scan-collections": { + "post": { + "tags": [ + "Emby" + ], + "summary": "Scan an Emby source's collections", + "description": "Queues a synchronization of the source's collections (fire-and-forget). Pass ?deep=true for a deep scan. Returns 409 while an Emby collections scan is already in progress.", + "operationId": "ScanEmbyCollections", + "parameters": [ + { + "name": "id", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int32" + } + }, + { + "name": "deep", + "in": "query", + "schema": { + "type": "boolean", + "default": false + } + } + ], + "responses": { + "202": { + "description": "Accepted" + }, + "404": { + "description": "Not Found", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, + "409": { + "description": "Conflict", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + } + } + } + }, "/api/ffmpeg/profiles": { "get": { "tags": [ @@ -6576,6 +6650,80 @@ } } }, + "/api/media-sources/jellyfin/{id}/scan-collections": { + "post": { + "tags": [ + "Jellyfin" + ], + "summary": "Scan a Jellyfin source's collections", + "description": "Queues a synchronization of the source's collections (fire-and-forget). Pass ?deep=true for a deep scan. Returns 409 while a Jellyfin collections scan is already in progress.", + "operationId": "ScanJellyfinCollections", + "parameters": [ + { + "name": "id", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int32" + } + }, + { + "name": "deep", + "in": "query", + "schema": { + "type": "boolean", + "default": false + } + } + ], + "responses": { + "202": { + "description": "Accepted" + }, + "404": { + "description": "Not Found", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, + "409": { + "description": "Conflict", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + } + } + } + }, "/api/languages": { "get": { "tags": [ @@ -6662,6 +6810,7 @@ "Libraries" ], "summary": "Scan library", + "description": "Queues a scan of the whole library. Pass ?deep=true for a deep (metadata-refresh) scan.", "parameters": [ { "name": "id", @@ -6671,6 +6820,14 @@ "type": "integer", "format": "int32" } + }, + { + "name": "deep", + "in": "query", + "schema": { + "type": "boolean", + "default": false + } } ], "responses": { @@ -6783,8 +6940,8 @@ "required": true }, "responses": { - "200": { - "description": "OK" + "202": { + "description": "Accepted" }, "404": { "description": "Not Found", @@ -6806,8 +6963,28 @@ } } }, - "400": { - "description": "Bad Request", + "409": { + "description": "Conflict", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, + "422": { + "description": "Unprocessable Entity", "content": { "text/plain": { "schema": { @@ -7548,6 +7725,26 @@ "responses": { "200": { "description": "OK" + }, + "422": { + "description": "Unprocessable Entity", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } } } } @@ -7559,8 +7756,8 @@ ], "summary": "Clean artwork cache", "responses": { - "200": { - "description": "OK" + "202": { + "description": "Accepted" } } } @@ -10458,7 +10655,24 @@ "operationId": "ResetAllPlayouts", "responses": { "202": { - "description": "Accepted" + "description": "Accepted", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ResetAllPlayoutsResponseModel" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ResetAllPlayoutsResponseModel" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ResetAllPlayoutsResponseModel" + } + } + } } } } @@ -11275,6 +11489,80 @@ } } }, + "/api/media-sources/plex/{id}/scan-collections": { + "post": { + "tags": [ + "Plex" + ], + "summary": "Scan a Plex server's collections", + "description": "Queues a synchronization of the server's collections (fire-and-forget). Pass ?deep=true for a deep scan. Returns 409 while a Plex collections scan is already in progress.", + "operationId": "ScanPlexCollections", + "parameters": [ + { + "name": "id", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int32" + } + }, + { + "name": "deep", + "in": "query", + "schema": { + "type": "boolean", + "default": false + } + } + ], + "responses": { + "202": { + "description": "Accepted" + }, + "404": { + "description": "Not Found", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, + "409": { + "description": "Conflict", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + } + } + } + }, "/api/rerun-collections": { "get": { "tags": [ @@ -15846,6 +16134,26 @@ } ], "responses": { + "404": { + "description": "Not Found", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "409": { "description": "Conflict", "content": { @@ -15865,6 +16173,26 @@ } } } + }, + "422": { + "description": "Unprocessable Entity", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } } } }, @@ -15960,6 +16288,26 @@ } ], "responses": { + "404": { + "description": "Not Found", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "409": { "description": "Conflict", "content": { @@ -15979,6 +16327,26 @@ } } } + }, + "422": { + "description": "Unprocessable Entity", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } } } } @@ -22533,7 +22901,8 @@ "dailyRebuildTime", "buildStatus", "decoId", - "decoName" + "decoName", + "isLocked" ], "type": "object", "properties": { @@ -22591,6 +22960,9 @@ "null", "string" ] + }, + "isLocked": { + "type": "boolean" } } }, @@ -23422,6 +23794,37 @@ } } }, + "ResetAllPlayoutsResponseModel": { + "required": [ + "queuedPlayoutIds", + "skippedLocked", + "skippedUnsupported" + ], + "type": "object", + "properties": { + "queuedPlayoutIds": { + "type": "array", + "items": { + "type": "integer", + "format": "int32" + } + }, + "skippedLocked": { + "type": "array", + "items": { + "type": "integer", + "format": "int32" + } + }, + "skippedUnsupported": { + "type": "array", + "items": { + "type": "integer", + "format": "int32" + } + } + } + }, "ResolutionResponseModel": { "required": [ "id", diff --git a/docs/endpoint-index.md b/docs/endpoint-index.md index d1b1ad3e5..7f833e36d 100644 --- a/docs/endpoint-index.md +++ b/docs/endpoint-index.md @@ -2,7 +2,7 @@ *Generated by `scripts/generate-endpoint-index.py` from `ErsatzTV/wwwroot/openapi/v1.json`. Do not edit by hand -- regenerated by `scripts/update-openapi.sh`.* -155 endpoints, 240 operations. +158 endpoints, 243 operations. ## Artists @@ -122,6 +122,7 @@ | GET | `/api/media-sources/emby/{id}/path-replacements` | GetEmbyPathReplacements | Get an Emby source's path replacements | | PUT | `/api/media-sources/emby/{id}/path-replacements` | ReplaceEmbyPathReplacements | Replace an Emby source's path replacements | | POST | `/api/media-sources/emby/{id}/refresh-libraries` | RefreshEmbyLibraries | Refresh an Emby source's libraries | +| POST | `/api/media-sources/emby/{id}/scan-collections` | ScanEmbyCollections | Scan an Emby source's collections | ## FFmpeg Profiles @@ -176,6 +177,7 @@ | GET | `/api/media-sources/jellyfin/{id}/path-replacements` | GetJellyfinPathReplacements | Get a Jellyfin source's path replacements | | PUT | `/api/media-sources/jellyfin/{id}/path-replacements` | ReplaceJellyfinPathReplacements | Replace a Jellyfin source's path replacements | | POST | `/api/media-sources/jellyfin/{id}/refresh-libraries` | RefreshJellyfinLibraries | Refresh a Jellyfin source's libraries | +| POST | `/api/media-sources/jellyfin/{id}/scan-collections` | ScanJellyfinCollections | Scan a Jellyfin source's collections | ## Languages @@ -295,6 +297,7 @@ | GET | `/api/media-sources/plex/{id}/path-replacements` | GetPlexPathReplacements | Get a Plex server's path replacements | | PUT | `/api/media-sources/plex/{id}/path-replacements` | ReplacePlexPathReplacements | Replace a Plex server's path replacements | | POST | `/api/media-sources/plex/{id}/refresh-libraries` | RefreshPlexLibraries | Refresh a Plex server's libraries | +| POST | `/api/media-sources/plex/{id}/scan-collections` | ScanPlexCollections | Scan a Plex server's collections | ## Rerun Collections diff --git a/web/src/api/generated/v1.d.ts b/web/src/api/generated/v1.d.ts index b03c5e51d..d4ee00744 100644 --- a/web/src/api/generated/v1.d.ts +++ b/web/src/api/generated/v1.d.ts @@ -1093,6 +1093,7 @@ export interface components { "buildStatus": null | components["schemas"]["PlayoutBuildStatusResponseModel"]; "decoId": null | number; "decoName": null | string; + "isLocked": boolean; }; "PlayoutScheduleKind": "None" | "Classic" | "Block" | "Sequential" | "Scripted" | "ExternalJson"; "PlayoutSettingsResponseModel": { @@ -1243,6 +1244,11 @@ export interface components { "selectedName": null | string; "firstRunPlaybackOrder": components["schemas"]["PlaybackOrder"]; "rerunPlaybackOrder": components["schemas"]["PlaybackOrder"]; + }; + "ResetAllPlayoutsResponseModel": { + "queuedPlayoutIds": Array; + "skippedLocked": Array; + "skippedUnsupported": Array; }; "ResolutionResponseModel": { "id": number; From d32ca976f72f881efe5d0426cd83977b7be4d56a Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 11 Jul 2026 18:10:02 +0200 Subject: [PATCH 08/12] feat(235): SPA clients for deep/collections scan + typed reset-all; docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - libraries.ts: scanLibrary(id, deep), new scanCollections(source, id, deep), corrected stale scanShow status-code comment (400 -> 202/404/409/422) - playouts.ts: resetAllPlayouts returns typed ResetAllPlayoutsResponseModel body - libraries.test.ts: deep-scan + scanCollections client tests - decisions.md: #235 async-op contract + F9 endpoints + accepted-by-design channels note - blazor-route-parity.md §5: F9 API gate closed; SPA deep/collections buttons = removal-PR work Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/blazor-route-parity.md | 6 +++- docs/decisions.md | 55 +++++++++++++++++++++++++++++++++++ web/src/api/libraries.test.ts | 21 ++++++++++++- web/src/api/libraries.ts | 28 ++++++++++++++---- web/src/api/playouts.ts | 9 ++++-- 5 files changed, 110 insertions(+), 9 deletions(-) diff --git a/docs/blazor-route-parity.md b/docs/blazor-route-parity.md index 1addc1409..ed95b8e5e 100644 --- a/docs/blazor-route-parity.md +++ b/docs/blazor-route-parity.md @@ -273,7 +273,11 @@ The removal PR is **gated** — it starts only after these clear: ~~#202 (media- **DONE 2026-07-11**, ~~#235 F9 (deep-scan + external-collections-scan API)~~ **API DONE (#235 slice B)**: `POST /api/libraries/{id}/scan?deep=` now threads deep-scan, and `POST /api/media-sources/{plex|jellyfin|emby}/{id}/scan-collections?deep=` covers external-collections scan (the two `Libraries.razor` parity gaps) — the SPA `Libraries.razor` port can now -proceed, and the **mandatory cold adversarial pass** (#91 comment 2026-07-09). (#204's id-carrying pattern +proceed, and the **mandatory cold adversarial pass** (#91 comment 2026-07-09). **Remaining SPA affordance for +the removal PR** (API + thin `web/src/api/libraries.ts` clients — `scanLibrary(id, deep)`, `scanCollections` — +already shipped by #235): `LibrariesScreen` currently exposes only quick-scan; the removal PR must add the +**deep-scan** and **external-collections-scan** buttons (wiring the shipped clients) before deleting +`Libraries.razor`, or that capability is lost. (#204's id-carrying pattern redirects landed 2026-07-11; its catch-all fallback that replaces `MapFallbackToPage("/_Host")` is folded into Step 2 below, since it can only ship when `_Host` is deleted.) When those close, the removal-PR routine executes, in order: diff --git a/docs/decisions.md b/docs/decisions.md index ce6d41b94..0d7a1d738 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -649,3 +649,58 @@ action of the Step 2 deletion PR merge** (not before — `main` moves until then cut. Not cut this session — `main` still carries Blazor and will advance before the removal PR. + +## 2026-07-11 — Async-op API contract normalization + playout build observability + F9 scan endpoints (#235) + +Reviewer#20 F7/F8/F9. Normalizes the queue-triggering `/api/*` endpoints onto one contract, closes the two +F9 `Libraries.razor` parity gaps, and hardens the Trakt batch-lock lifecycle. Much of the F8 surface was +**already normalized** by #232 (library scan → `QueueLibraryScanResult` 202/404/409/422) and #215 (per-id +playout mutations + reset → 409 lock guard) — this issue finished the remaining outliers. + +**Normalized async-op contract** (queue-triggering endpoints): **202 Accepted** = work queued; **404 +ProblemDetails** = entity missing (controller pre-check); **409 ProblemDetails** = lock held (the running +job, or a mutation racing it — §3a/§3b); **422 ProblemDetails** = domain precondition (sync disabled / +unsupported / start failed). Trakt was the reference implementation. Changes made: +- `MaintenanceController.EmptyTrash` — error path **500 text/plain → 404/422 ProblemDetails** (`ToErrorResult`). +- `MaintenanceController.CleanArtwork` — silent **200 → 202** (fire-and-forget enqueue). No SPA consumer. +- `LibrariesController.ScanShow` — conflated **400 `{error}` → 202/404/409/422** via a new + `QueueShowScanResult` enum (6 outcomes incl. an honest `ScanFailed`→422, distinct from `Unsupported`). +- `ChannelController.ResetPlayout` — **200 → 202** (queue-triggering; 404/409 unchanged). +- `PlayoutController.ResetAll` — **202 (no body) → 202 + `ResetAllPlayoutsResponseModel`** reporting + `queuedPlayoutIds` / `skippedLocked` / `skippedUnsupported` (replaces the silent skip; still 202, still + skips locked/ExternalJson by design per §3a — now it *reports* what it skipped). +- `TroubleshootController.TroubleshootPlayback` — bare body-less `NotFound()` → **404/422 ProblemDetails** + with distinguishing detail. **Status codes the SPA HLS player depends on were preserved** — verified + `HlsPlayer.tsx` never branches on this endpoint's status (playback state comes from the separate + `/api/troubleshoot/playback/status` poll); only the error *body* was enriched. + +**Playout build observability**: the list endpoint (`GET /api/playouts`) already stamped `isLocked` + +`BuildStatus` on `PlayoutListItemResponseModel` (#215); this issue adds **`isLocked` to the single-playout +`GET /api/playouts/{id}`** (`PlayoutResponseModel`), so the detail poll surface carries the §3a lock flag +too. No dedicated `GET /api/playouts/{id}/status` push channel was added — the flag on the existing GETs is +the HTTP-observable substitute for Blazor's live lock event, matching the `GET /api/trakt/status` precedent. + +**F9 parity endpoints** (the `Libraries.razor` deletion gate — #202 did NOT close these): +- **Deep scan**: `POST /api/libraries/{id}/scan` gains `?deep=false`, threaded through + `QueueLibraryScanByLibraryId(LibraryId, DeepScan=false)` into `ForceSynchronize{Plex,Jellyfin,Emby}LibraryById(id, deep)` + (was hardcoded `false`). Non-breaking: existing callers omit it. +- **External-collections scan**: new `POST /api/media-sources/{plex|jellyfin|emby}/{id}/scan-collections?deep=false` + on the three #202 media-source controllers, dispatching `Synchronize{X}Collections(id, ForceScan:true, deep)`. + Each pre-checks source existence (404), acquires the per-source **collections** lock (`Lock{X}Collections()` — + the lock *is* the running scan, so a false = **409**), then enqueues and returns 202; the controller + compensating-unlocks in a `catch` if the enqueue throws (§3b), and `ScannerService` releases in its `finally`. + Thin SPA clients shipped (`scanLibrary(id, deep)`, `scanCollections`); **the SPA deep-scan / collections + buttons are the removal PR's remaining parity work** (parity doc §5). + +**F7 Trakt batch-lock leak fix**: the global Trakt lock was released only when the *terminal* batch message +(`Unlock: true`) was processed; a `WorkerService` shutdown/cancellation before that message leaked the lock +permanently (subsequent Trakt ops 409 until restart — same class as #231/#233/#234). Fix: `WorkerService` +now releases the Trakt lock in a `finally` on read-loop exit if still held. Non-vacuous regression test proven +against an inverted-condition control. + +**Accepted-by-design** (per the issue's decision-record ask): the worker's channels are **unbounded** and +there is **no shutdown drain** — messages still queued at process exit are dropped. This is acceptable because +the entity locks are **in-memory singletons that die with the process**, so a dropped message can't strand a +lock across restarts (the F7 `finally` covers the *within-process* shutdown-break leak, which is the only way +a lock outlives its batch while the process keeps running). Adding a bounded-channel backpressure / graceful +drain is out of scope and would not fix a correctness bug. diff --git a/web/src/api/libraries.test.ts b/web/src/api/libraries.test.ts index 349f26bb0..2cb3745cd 100644 --- a/web/src/api/libraries.test.ts +++ b/web/src/api/libraries.test.ts @@ -1,5 +1,5 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { scanLibrary, scanShow } from './libraries'; +import { scanCollections, scanLibrary, scanShow } from './libraries'; function noContent(): Response { return new Response(null, { status: 200 }); @@ -22,6 +22,25 @@ describe('libraries api client', () => { expect(fetchMock).toHaveBeenCalledWith('/api/libraries/4/scan', expect.objectContaining({ method: 'POST' })); }); + it('scanLibrary appends ?deep=true for a deep scan', async () => { + const fetchMock = vi.spyOn(window, 'fetch').mockResolvedValue(noContent()); + await scanLibrary(4, true); + expect(fetchMock).toHaveBeenCalledWith('/api/libraries/4/scan?deep=true', expect.objectContaining({ method: 'POST' })); + }); + + it('scanCollections POSTs to the media-source scan-collections endpoint', async () => { + const fetchMock = vi.spyOn(window, 'fetch').mockResolvedValue(noContent()); + await scanCollections('jellyfin', 7); + expect(fetchMock).toHaveBeenCalledWith('/api/media-sources/jellyfin/7/scan-collections', expect.objectContaining({ method: 'POST' })); + }); + + it('scanCollections appends ?deep=true for a deep scan', async () => { + const fetchMock = vi.spyOn(window, 'fetch').mockResolvedValue(noContent()); + await scanCollections('plex', 3, true); + const { url } = lastCall(fetchMock); + expect(url).toBe('/api/media-sources/plex/3/scan-collections?deep=true'); + }); + it('scanShow POSTs the show id and deepScan flag', async () => { const fetchMock = vi.spyOn(window, 'fetch').mockResolvedValue(noContent()); await scanShow(4, { deepScan: true, showId: 42 }); diff --git a/web/src/api/libraries.ts b/web/src/api/libraries.ts index 66361eae9..97b66ad98 100644 --- a/web/src/api/libraries.ts +++ b/web/src/api/libraries.ts @@ -42,8 +42,24 @@ export function getLibraryScanStatus(): Promise { ); } -export function scanLibrary(libraryId: number): Promise { - return request(`/api/libraries/${libraryId}/scan`, { method: 'POST' }); +// Queues a library scan. `deep` requests a deep (force-metadata) scan — the API equivalent of the +// legacy Blazor "Deep Scan Library" button (#235 F9); omit or pass false for a quick scan. Returns +// 202 when queued, 404 (missing), 409 (already scanning), 422 (sync disabled) — see §3b. +export function scanLibrary(libraryId: number, deep = false): Promise { + const query = deep ? '?deep=true' : ''; + return request(`/api/libraries/${libraryId}/scan${query}`, { method: 'POST' }); +} + +// Media-source families that support an external-collections scan (#235 F9). Matches the three +// media-source controllers #202 introduced. +export type CollectionsScanSource = 'emby' | 'jellyfin' | 'plex'; + +// Queues an external-collections scan for a Plex/Jellyfin/Emby media source — the API equivalent of +// the legacy Blazor "Scan Collections" button (#235 F9). `deep` requests a deep scan. Returns 202 +// when queued, 404 (source missing), 409 (a collections scan is already running for that source). +export function scanCollections(source: CollectionsScanSource, sourceId: number, deep = false): Promise { + const query = deep ? '?deep=true' : ''; + return request(`/api/media-sources/${source}/${sourceId}/scan-collections${query}`, { method: 'POST' }); } export interface ScanShowParams { @@ -51,9 +67,11 @@ export interface ScanShowParams { deepScan?: boolean; } -// Queues a scan of a single show (by id) within a library. Returns 200 on success, 404 when the -// show id doesn't exist in the library, 400 when the library doesn't support single-show -// scanning. Body keys are `showId` and `deepScan` (see LibrariesController.ScanShowRequest). +// Queues a scan of a single show (by id) within a library. Returns 202 when queued, 404 when the +// show id doesn't exist in the library, 409 when a scan is already running, and 422 when the +// library doesn't support single-show scanning / sync is disabled / the scan failed to start (all +// error bodies are ProblemDetails; #235 normalized the old conflated 400). Body keys are `showId` +// and `deepScan` (see LibrariesController.ScanShowRequest). export function scanShow(libraryId: number, params: ScanShowParams): Promise { return request(`/api/libraries/${libraryId}/scan-show`, { body: { deepScan: params.deepScan ?? false, showId: params.showId }, diff --git a/web/src/api/playouts.ts b/web/src/api/playouts.ts index 839281c3d..529800061 100644 --- a/web/src/api/playouts.ts +++ b/web/src/api/playouts.ts @@ -108,8 +108,13 @@ export function getPlayoutChannelStates(): Promise { return request('/api/channels/state'); } -export function resetAllPlayouts(): Promise { - return request('/api/playouts/reset-all', { method: 'POST' }); +export type ResetAllPlayoutsResult = components['schemas']['ResetAllPlayoutsResponseModel']; + +// Queues a reset of every eligible playout. Returns 202 with a body reporting which playouts were +// queued vs skipped (locked, or an unsupported ExternalJson/None schedule kind) — #235 replaced the +// old silent skip. The caller may surface `skipped*` to explain why some playouts didn't reset. +export function resetAllPlayouts(): Promise { + return request('/api/playouts/reset-all', { method: 'POST' }); } export function deletePlayout(playoutId: number): Promise { From 787058d18c049cd3dde0ba8ae3b1c5e6c29e1cb0 Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 11 Jul 2026 18:37:35 +0200 Subject: [PATCH 09/12] fix(235): scheduler-safe collections lock ownership (Codex High / Fable reconciliation) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The new POST /api/media-sources/{plex|jellyfin|emby}/{id}/scan-collections endpoints acquire a per-provider collections lock (409 if held) and hand the single release to the ScannerService finally. But SchedulerService's periodic collection scans were enqueued WITHOUT the lock, and ScannerService's finally released the collections lock whenever held with no ownership check. A scheduler-queued scan running while an API request held the lock cross-released the API's lock (#250 bug class), letting a second API request get a spurious 202 instead of 409. Fix (mirrors the SynchronizePlexLibraryByIdIfNeeded(Unlock: !networksFollow) library-scan precedent): - Add `bool Unlock = true` (4th positional param) to the three Synchronize{Plex,Jellyfin,Emby}Collections records; default keeps the controller + Libraries.razor call sites compiling and releasing on run. - ScannerService: the three collection finallys now honor `request.Unlock` (the concrete typed request is in scope in each method) so a batch member with Unlock:false never releases a lock it doesn't own. - SchedulerService: replace the unlocked per-source enqueue with a lock-once per-provider batch — LockX Collections() once, enqueue each source with Unlock:isLast (last message owns the release), compensating unlock in catch, and SKIP the whole provider loop if the lock is already held. A naive "lock-per-source, skip if held" would deterministically starve the 2nd+ source; lock-once-batch does not. Tests (ErsatzTV.Tests/Services/): ScannerServiceCollectionLockTests drives the real ScannerService read loop + real EntityLocker and asserts Unlock:false leaves a held lock intact while Unlock:true releases (all three providers); SchedulerServiceCollectionLockTests reflect-invokes ScanPlexMediaSources and asserts it locks once + skips the enqueue when held, and hands the release to the last message when acquired. Proven non-vacuous: reverting the Plex fix fails exactly the three Plex tests. No OpenAPI/v1.json change (internal channel-message record, not a DTO). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Commands/SynchronizeEmbyCollections.cs | 2 +- .../SynchronizeJellyfinCollections.cs | 2 +- .../Commands/SynchronizePlexCollections.cs | 2 +- .../ScannerServiceCollectionLockTests.cs | 160 ++++++++++++++++++ .../SchedulerServiceCollectionLockTests.cs | 150 ++++++++++++++++ ErsatzTV/Services/ScannerService.cs | 14 +- ErsatzTV/Services/SchedulerService.cs | 76 +++++++-- 7 files changed, 388 insertions(+), 18 deletions(-) create mode 100644 ErsatzTV.Tests/Services/ScannerServiceCollectionLockTests.cs create mode 100644 ErsatzTV.Tests/Services/SchedulerServiceCollectionLockTests.cs diff --git a/ErsatzTV.Application/Emby/Commands/SynchronizeEmbyCollections.cs b/ErsatzTV.Application/Emby/Commands/SynchronizeEmbyCollections.cs index b6511c5d1..f9b9c8ce4 100644 --- a/ErsatzTV.Application/Emby/Commands/SynchronizeEmbyCollections.cs +++ b/ErsatzTV.Application/Emby/Commands/SynchronizeEmbyCollections.cs @@ -2,6 +2,6 @@ using ErsatzTV.Core; namespace ErsatzTV.Application.Emby; -public record SynchronizeEmbyCollections(int EmbyMediaSourceId, bool ForceScan, bool DeepScan) +public record SynchronizeEmbyCollections(int EmbyMediaSourceId, bool ForceScan, bool DeepScan, bool Unlock = true) : IRequest>, IScannerBackgroundServiceRequest; diff --git a/ErsatzTV.Application/Jellyfin/Commands/SynchronizeJellyfinCollections.cs b/ErsatzTV.Application/Jellyfin/Commands/SynchronizeJellyfinCollections.cs index 705bd8c03..fc38efe68 100644 --- a/ErsatzTV.Application/Jellyfin/Commands/SynchronizeJellyfinCollections.cs +++ b/ErsatzTV.Application/Jellyfin/Commands/SynchronizeJellyfinCollections.cs @@ -2,6 +2,6 @@ using ErsatzTV.Core; namespace ErsatzTV.Application.Jellyfin; -public record SynchronizeJellyfinCollections(int JellyfinMediaSourceId, bool ForceScan, bool DeepScan) : +public record SynchronizeJellyfinCollections(int JellyfinMediaSourceId, bool ForceScan, bool DeepScan, bool Unlock = true) : IRequest>, IScannerBackgroundServiceRequest; diff --git a/ErsatzTV.Application/Plex/Commands/SynchronizePlexCollections.cs b/ErsatzTV.Application/Plex/Commands/SynchronizePlexCollections.cs index b081770c9..ddefbcb63 100644 --- a/ErsatzTV.Application/Plex/Commands/SynchronizePlexCollections.cs +++ b/ErsatzTV.Application/Plex/Commands/SynchronizePlexCollections.cs @@ -2,5 +2,5 @@ using ErsatzTV.Core; namespace ErsatzTV.Application.Plex; -public record SynchronizePlexCollections(int PlexMediaSourceId, bool ForceScan, bool DeepScan) +public record SynchronizePlexCollections(int PlexMediaSourceId, bool ForceScan, bool DeepScan, bool Unlock = true) : IRequest>, IScannerBackgroundServiceRequest; diff --git a/ErsatzTV.Tests/Services/ScannerServiceCollectionLockTests.cs b/ErsatzTV.Tests/Services/ScannerServiceCollectionLockTests.cs new file mode 100644 index 000000000..0bbb04539 --- /dev/null +++ b/ErsatzTV.Tests/Services/ScannerServiceCollectionLockTests.cs @@ -0,0 +1,160 @@ +using System.Reflection; +using System.Threading.Channels; +using ErsatzTV.Application; +using ErsatzTV.Application.Emby; +using ErsatzTV.Application.Jellyfin; +using ErsatzTV.Application.Plex; +using ErsatzTV.Core; +using ErsatzTV.Core.Interfaces.Locking; +using ErsatzTV.Infrastructure.Locking; +using ErsatzTV.Services; +using LanguageExt; +using MediatR; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; +using NUnit.Framework; +using Shouldly; + +namespace ErsatzTV.Tests.Services; + +/// +/// Regression net for the #250 cross-release bug on the per-provider collections lock (#235): +/// a scheduler-enqueued collection scan running with Unlock: false must NOT release a +/// collections lock owned by another party (an in-flight API scan), while Unlock: true +/// (an API request or the last message of a scheduler batch) must release on completion. +/// +[TestFixture] +public class ScannerServiceCollectionLockTests +{ + [Test] + public async Task Plex_UnlockFalse_DoesNotReleaseHeldLock() + { + (ScannerService service, EntityLocker locker) = BuildService( + new SynchronizePlexCollections(1, false, false, Unlock: false)); + + // another party (an API scan) owns the collections lock + locker.LockPlexCollections().ShouldBeTrue(); + + await RunToCompletion(service); + + locker.ArePlexCollectionsLocked().ShouldBeTrue(); + } + + [Test] + public async Task Plex_UnlockTrue_ReleasesHeldLock() + { + (ScannerService service, EntityLocker locker) = BuildService( + new SynchronizePlexCollections(1, false, false, Unlock: true)); + + locker.LockPlexCollections().ShouldBeTrue(); + + await RunToCompletion(service); + + locker.ArePlexCollectionsLocked().ShouldBeFalse(); + } + + [Test] + public async Task Jellyfin_UnlockFalse_DoesNotReleaseHeldLock() + { + (ScannerService service, EntityLocker locker) = BuildService( + new SynchronizeJellyfinCollections(1, false, false, Unlock: false)); + + locker.LockJellyfinCollections().ShouldBeTrue(); + + await RunToCompletion(service); + + locker.AreJellyfinCollectionsLocked().ShouldBeTrue(); + } + + [Test] + public async Task Jellyfin_UnlockTrue_ReleasesHeldLock() + { + (ScannerService service, EntityLocker locker) = BuildService( + new SynchronizeJellyfinCollections(1, false, false, Unlock: true)); + + locker.LockJellyfinCollections().ShouldBeTrue(); + + await RunToCompletion(service); + + locker.AreJellyfinCollectionsLocked().ShouldBeFalse(); + } + + [Test] + public async Task Emby_UnlockFalse_DoesNotReleaseHeldLock() + { + (ScannerService service, EntityLocker locker) = BuildService( + new SynchronizeEmbyCollections(1, false, false, Unlock: false)); + + locker.LockEmbyCollections().ShouldBeTrue(); + + await RunToCompletion(service); + + locker.AreEmbyCollectionsLocked().ShouldBeTrue(); + } + + [Test] + public async Task Emby_UnlockTrue_ReleasesHeldLock() + { + (ScannerService service, EntityLocker locker) = BuildService( + new SynchronizeEmbyCollections(1, false, false, Unlock: true)); + + locker.LockEmbyCollections().ShouldBeTrue(); + + await RunToCompletion(service); + + locker.AreEmbyCollectionsLocked().ShouldBeFalse(); + } + + private static (ScannerService, EntityLocker) BuildService(IScannerBackgroundServiceRequest request) + { + var mediator = Substitute.For(); + // every collection sync request returns success; the finally is what we're exercising + Either ok = LanguageExt.Unit.Default; + mediator.Send(Arg.Any(), Arg.Any()) + .Returns(ok); + mediator.Send(Arg.Any(), Arg.Any()) + .Returns(ok); + mediator.Send(Arg.Any(), Arg.Any()) + .Returns(ok); + + var locker = new EntityLocker(mediator, NullLogger.Instance); + + var provider = Substitute.For(); + provider.GetService(typeof(IMediator)).Returns(mediator); + provider.GetService(typeof(IEntityLocker)).Returns(locker); + + var scope = Substitute.For(); + scope.ServiceProvider.Returns(provider); + + var scopeFactory = Substitute.For(); + scopeFactory.CreateScope().Returns(scope); + + var channel = Channel.CreateUnbounded(); + channel.Writer.TryWrite(request).ShouldBeTrue(); + channel.Writer.Complete(); + + var startup = new SystemStartup(); + startup.DatabaseIsReady(); + startup.SearchIndexIsReady(); + + var service = new ScannerService( + channel.Reader, + scopeFactory, + startup, + NullLogger.Instance); + + return (service, locker); + } + + private static async Task RunToCompletion(ScannerService service) + { + MethodInfo executeAsync = typeof(ScannerService) + .GetMethod("ExecuteAsync", BindingFlags.NonPublic | BindingFlags.Instance)!; + + // the channel writer is already completed, so the read loop drains the one queued + // request, runs its finally, then ExecuteAsync returns + var task = (Task)executeAsync.Invoke(service, [CancellationToken.None])!; + await task.WaitAsync(TimeSpan.FromSeconds(10)); + } +} diff --git a/ErsatzTV.Tests/Services/SchedulerServiceCollectionLockTests.cs b/ErsatzTV.Tests/Services/SchedulerServiceCollectionLockTests.cs new file mode 100644 index 000000000..68c229d88 --- /dev/null +++ b/ErsatzTV.Tests/Services/SchedulerServiceCollectionLockTests.cs @@ -0,0 +1,150 @@ +using System.Reflection; +using System.Threading.Channels; +using SysChannels = System.Threading.Channels; +using ErsatzTV.Application; +using ErsatzTV.Application.Plex; +using ErsatzTV.Core; +using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Interfaces.Locking; +using ErsatzTV.Infrastructure.Data; +using ErsatzTV.Services; +using ErsatzTV.Tests.Support; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; +using NUnit.Framework; +using Shouldly; + +namespace ErsatzTV.Tests.Services; + +/// +/// Verifies the scheduler's lock-once-per-provider collection batch (#235): it acquires +/// LockPlexCollections() exactly once, and when the slot is already held (an API scan or a +/// prior tick) it SKIPS the collection enqueue entirely rather than enqueuing an unlocked scan +/// that would cross-release the holder's lock (#250). When it does acquire, only the LAST message +/// carries the release (Unlock: true). +/// +[TestFixture] +public class SchedulerServiceCollectionLockTests +{ + private InMemoryTvContext _db; + + [SetUp] + public async Task SetUp() => _db = await InMemoryTvContext.CreateAsync(); + + [TearDown] + public async Task TearDown() => await _db.DisposeAsync(); + + [Test] + public async Task Plex_SkipsCollectionEnqueue_WhenCollectionsLockAlreadyHeld() + { + await SeedPlexLibrary(id: 1, mediaSourceId: 7); + + var locker = Substitute.For(); + // isolate the collection path: the library loop enqueues nothing but still records the source id + locker.LockLibrary(Arg.Any()).Returns(false); + // the collections slot is already held by another party + locker.LockPlexCollections().Returns(false); + + (SchedulerService service, SysChannels.Channel scannerChannel) = BuildService(locker); + + await InvokeScanPlex(service); + List writes = await Drain(scannerChannel); + + locker.Received(1).LockPlexCollections(); + writes.OfType().ShouldBeEmpty(); + locker.DidNotReceive().UnlockPlexCollections(); + } + + [Test] + public async Task Plex_EnqueuesBatch_LastMessageOwnsRelease_WhenLockAcquired() + { + await SeedPlexLibrary(id: 1, mediaSourceId: 7); + await SeedPlexLibrary(id: 2, mediaSourceId: 8); + + var locker = Substitute.For(); + locker.LockLibrary(Arg.Any()).Returns(false); + locker.LockPlexCollections().Returns(true); + + (SchedulerService service, SysChannels.Channel scannerChannel) = BuildService(locker); + + await InvokeScanPlex(service); + List writes = await Drain(scannerChannel); + + locker.Received(1).LockPlexCollections(); + + List collectionScans = writes.OfType().ToList(); + collectionScans.Count.ShouldBe(2); + // exactly one message owns the single release, and it is the last one written + collectionScans.Count(m => m.Unlock).ShouldBe(1); + collectionScans.Last().Unlock.ShouldBeTrue(); + collectionScans.First().Unlock.ShouldBeFalse(); + + // release is handed off to the last message; no compensating unlock on the happy path + locker.DidNotReceive().UnlockPlexCollections(); + } + + private async Task SeedPlexLibrary(int id, int mediaSourceId) + { + await using TvContext context = _db.CreateContext(); + context.PlexLibraries.Add( + new PlexLibrary + { + Id = id, + Name = $"Plex {id}", + MediaKind = LibraryMediaKind.Movies, + MediaSourceId = mediaSourceId, + ShouldSyncItems = true, + Key = $"key-{id}" + }); + await context.SaveChangesAsync(); + } + + private (SchedulerService, SysChannels.Channel) BuildService(IEntityLocker locker) + { + var provider = Substitute.For(); + provider.GetService(typeof(TvContext)).Returns(_ => _db.CreateContext()); + + var scope = Substitute.For(); + scope.ServiceProvider.Returns(provider); + + var scopeFactory = Substitute.For(); + scopeFactory.CreateScope().Returns(scope); + + var workerChannel = SysChannels.Channel.CreateUnbounded(); + var scannerChannel = SysChannels.Channel.CreateUnbounded(); + + var startup = new SystemStartup(); + + var service = new SchedulerService( + scopeFactory, + workerChannel.Writer, + scannerChannel.Writer, + locker, + startup, + NullLogger.Instance); + + return (service, scannerChannel); + } + + private static async Task> Drain( + SysChannels.Channel channel) + { + channel.Writer.Complete(); + var results = new List(); + await foreach (IScannerBackgroundServiceRequest request in channel.Reader.ReadAllAsync()) + { + results.Add(request); + } + + return results; + } + + private static async Task InvokeScanPlex(SchedulerService service) + { + MethodInfo method = typeof(SchedulerService) + .GetMethod("ScanPlexMediaSources", BindingFlags.NonPublic | BindingFlags.Instance)!; + var task = (Task)method.Invoke(service, [CancellationToken.None])!; + await task.WaitAsync(TimeSpan.FromSeconds(10)); + } +} diff --git a/ErsatzTV/Services/ScannerService.cs b/ErsatzTV/Services/ScannerService.cs index cafd5380b..89271681a 100644 --- a/ErsatzTV/Services/ScannerService.cs +++ b/ErsatzTV/Services/ScannerService.cs @@ -221,7 +221,11 @@ public class ScannerService : BackgroundService } finally { - if (entityLocker.ArePlexCollectionsLocked()) + // request.Unlock is false when this collections scan is part of a scheduler-enqueued + // per-provider batch and a later message owns the single release (see #250 / #235). + // request.Unlock is false when this collections scan is part of a scheduler-enqueued + // per-provider batch and a later message owns the single release (see #250 / #235). + if (request.Unlock && entityLocker.ArePlexCollectionsLocked()) { entityLocker.UnlockPlexCollections(); } @@ -351,7 +355,9 @@ public class ScannerService : BackgroundService } finally { - if (entityLocker.AreJellyfinCollectionsLocked()) + // request.Unlock is false when this collections scan is part of a scheduler-enqueued + // per-provider batch and a later message owns the single release (see #250 / #235). + if (request.Unlock && entityLocker.AreJellyfinCollectionsLocked()) { entityLocker.UnlockJellyfinCollections(); } @@ -438,7 +444,9 @@ public class ScannerService : BackgroundService } finally { - if (entityLocker.AreEmbyCollectionsLocked()) + // request.Unlock is false when this collections scan is part of a scheduler-enqueued + // per-provider batch and a later message owns the single release (see #250 / #235). + if (request.Unlock && entityLocker.AreEmbyCollectionsLocked()) { entityLocker.UnlockEmbyCollections(); } diff --git a/ErsatzTV/Services/SchedulerService.cs b/ErsatzTV/Services/SchedulerService.cs index 0845bf6fa..de81a92a4 100644 --- a/ErsatzTV/Services/SchedulerService.cs +++ b/ErsatzTV/Services/SchedulerService.cs @@ -265,11 +265,35 @@ public class SchedulerService : BackgroundService } } - foreach (int mediaSourceId in mediaSourceIds) + // lock the collections slot once for the whole provider batch: the LAST message owns the + // single release (Unlock: true), the rest run Unlock: false. If the slot is already held + // (an API scan or a prior tick's batch is running/queued), skip enqueuing entirely - never + // enqueue unlocked, which would cross-release the holder's lock (#250). Skipping starves + // nothing: the next tick retries. (see #235) + // lock the collections slot once for the whole provider batch: the LAST message owns the + // single release (Unlock: true), the rest run Unlock: false. If the slot is already held + // (an API scan or a prior tick's batch is running/queued), skip enqueuing entirely - never + // enqueue unlocked, which would cross-release the holder's lock (#250). Skipping starves + // nothing: the next tick retries. (see #235) + var plexSourceIds = mediaSourceIds.ToList(); + if (plexSourceIds.Count > 0 && _entityLocker.LockPlexCollections()) { - await _scannerWorkerChannel.WriteAsync( - new SynchronizePlexCollections(mediaSourceId, false, false), - cancellationToken); + try + { + for (var i = 0; i < plexSourceIds.Count; i++) + { + bool isLast = i == plexSourceIds.Count - 1; + await _scannerWorkerChannel.WriteAsync( + new SynchronizePlexCollections(plexSourceIds[i], false, false, Unlock: isLast), + cancellationToken); + } + } + catch + { + // an enqueue threw before the last (releasing) message went out - compensate here + _entityLocker.UnlockPlexCollections(); + throw; + } } } @@ -300,11 +324,25 @@ public class SchedulerService : BackgroundService } } - foreach (int mediaSourceId in mediaSourceIds) + // lock-once per provider batch; last message owns the release; skip entirely if held (#235/#250) + var jellyfinSourceIds = mediaSourceIds.ToList(); + if (jellyfinSourceIds.Count > 0 && _entityLocker.LockJellyfinCollections()) { - await _scannerWorkerChannel.WriteAsync( - new SynchronizeJellyfinCollections(mediaSourceId, false, false), - cancellationToken); + try + { + for (var i = 0; i < jellyfinSourceIds.Count; i++) + { + bool isLast = i == jellyfinSourceIds.Count - 1; + await _scannerWorkerChannel.WriteAsync( + new SynchronizeJellyfinCollections(jellyfinSourceIds[i], false, false, Unlock: isLast), + cancellationToken); + } + } + catch + { + _entityLocker.UnlockJellyfinCollections(); + throw; + } } } @@ -335,11 +373,25 @@ public class SchedulerService : BackgroundService } } - foreach (int mediaSourceId in mediaSourceIds) + // lock-once per provider batch; last message owns the release; skip entirely if held (#235/#250) + var embySourceIds = mediaSourceIds.ToList(); + if (embySourceIds.Count > 0 && _entityLocker.LockEmbyCollections()) { - await _scannerWorkerChannel.WriteAsync( - new SynchronizeEmbyCollections(mediaSourceId, false, false), - cancellationToken); + try + { + for (var i = 0; i < embySourceIds.Count; i++) + { + bool isLast = i == embySourceIds.Count - 1; + await _scannerWorkerChannel.WriteAsync( + new SynchronizeEmbyCollections(embySourceIds[i], false, false, Unlock: isLast), + cancellationToken); + } + } + catch + { + _entityLocker.UnlockEmbyCollections(); + throw; + } } } From 589c35d3573a9e720a8263578f396588843268af Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 11 Jul 2026 18:41:33 +0200 Subject: [PATCH 10/12] fix(235): make QueueShowScanResult switch total (explicit NotFound + throwing fallback) Codex-review Low: NotFound previously reached the catch-all by coincidence; a future enum value would silently 404. Explicit arm + UnreachableException fallback so an unmapped outcome fails loudly rather than mis-mapping to 404. Co-Authored-By: Claude Opus 4.8 (1M context) --- ErsatzTV/Controllers/Api/LibrariesController.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ErsatzTV/Controllers/Api/LibrariesController.cs b/ErsatzTV/Controllers/Api/LibrariesController.cs index d75a11a5e..bd37c0bca 100644 --- a/ErsatzTV/Controllers/Api/LibrariesController.cs +++ b/ErsatzTV/Controllers/Api/LibrariesController.cs @@ -1,3 +1,4 @@ +using System.Diagnostics; using ErsatzTV.Application.Libraries; using ErsatzTV.Core.Api.Libraries; using ErsatzTV.Core.Interfaces.Repositories; @@ -93,7 +94,8 @@ public class LibrariesController(ITelevisionRepository televisionRepository, IMe Title = "Unable to scan show", Detail = $"The scan for show {request.ShowId} in library {id} could not be completed." }), - _ => ApiResults.NotFoundProblem($"Library {id} does not exist.") + QueueShowScanResult.NotFound => ApiResults.NotFoundProblem($"Library {id} does not exist."), + _ => throw new UnreachableException($"Unmapped QueueShowScanResult: {result}") }; } From 3981abc7f91e9e32d01c1be5d38616a7e1bac4b1 Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 11 Jul 2026 19:07:29 +0200 Subject: [PATCH 11/12] =?UTF-8?q?docs(handoff):=20lore=20=E2=80=94=20enume?= =?UTF-8?q?rate=20ALL=20channel/lock=20producers=20before=20a=20no-cross-r?= =?UTF-8?q?elease=20verdict=20(#235/#267)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/handoffs/chicorytv-issue-queue.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/docs/handoffs/chicorytv-issue-queue.md b/docs/handoffs/chicorytv-issue-queue.md index 596aa1692..409c11258 100644 --- a/docs/handoffs/chicorytv-issue-queue.md +++ b/docs/handoffs/chicorytv-issue-queue.md @@ -213,3 +213,19 @@ HARD CONSTRAINTS: on #N", status tables), not just resolve the conflict markers — git fast-forwards stale *facts* silently. The hand-written-docs sibling of the "regenerate generated artifacts after merging main" lesson. Also: docs-only PRs still trip main's `Build & test` branch protection, so a rebase = a fresh full-CI cycle before merge. +- **A lock/channel-ownership "no cross-release" verdict must enumerate EVERY producer of that message + type, not just the enqueuers visible in the PR diff** (2026-07-11, #235/#267). The cold fork cleared the + new `scan-collections` per-provider lock as sound — "only the acquirer enqueues, so only its message + triggers the scanner unlock" — because it checked the three new controllers but never grepped for ALL + `Synchronize{X}Collections(` senders. Codex did: `SchedulerService` also enqueues them *periodically and + unlocked*, and `ScannerService`'s `finally` released the lock whenever held → a scheduled scan + cross-releases an API scan's lock (#250 class). Rule: before trusting any "single owner / no double + release / no cross-release" claim, `grep` the whole host project for every writer of that channel message + (or acquirer of that lock) — the background scheduler/worker is the usual missing producer. This is why the + independent Codex pass earns its keep alongside the fork even when the fork returns MERGEABLE. +- **Fork-vs-Codex disagreement on a gate PR → Fable, and Fable often improves the fix, not just the verdict** + (2026-07-11, #235): the reconciliation isn't only "who's right" — Fable ratified Codex's High AND caught + that the orchestrator's proposed *fix* (per-source lock-or-skip) would deterministically starve the 2nd+ + media source (fixed loop order), prescribing the shipped library-scan §C7 pattern instead (lock once per + provider, batch-enqueue, release on the `Unlock: isLast` message). Escalate the FIX approach too, not just + the finding. From bb7b18992961f464d7a34eb172caef698396358a Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 11 Jul 2026 19:25:56 +0200 Subject: [PATCH 12/12] feat(91b): wire deep-scan + external-collections buttons into LibrariesScreen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Last SPA pre-work before deleting Blazor Libraries.razor (#91 phase b): wire the shipped scanLibrary(id, deep) + scanCollections(family, id, deep) clients (F9 API, #235) into LibrariesScreen so the SPA reaches parity with Libraries.razor's four scan actions. - Deep Scan Library button on each remote (Plex/Jellyfin/Emby) library row, threading `deep` through the existing optimistic-pending/poll hook (quick + deep share the per-library lock). - External Collections section (quick + deep per remote source). Rows derive client-side from getMediaSources(): the media-sources API handler already filters each source's `libraries` to sync-enabled entries, so a remote source with a non-empty libraries list is exactly GetExternalCollections's Libraries.Any(ShouldSyncItems) filter — no new endpoint. - useCollectionsScan hook: collections scans have no scan-status poll surface (the endpoint is library-keyed; Blazor observed collections locks via in-process IEntityLocker events), so pending is optimistic + timeout-bounded (409 benign, 404/network surfaces the error). Follow-up #271 for a proper collections status surface. Pure SPA change (no backend/OpenAPI). Docs: blazor-route-parity.md §5 (SPA affordance DONE), decisions.md (derive-vs-endpoint + optimistic-timeout). Refs #91 Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/blazor-route-parity.md | 22 ++-- docs/decisions.md | 26 ++++ web/src/api/libraries.ts | 110 +++++++++++++++- web/src/screens/LibrariesScreen.test.tsx | 156 ++++++++++++++++++++++- web/src/screens/LibrariesScreen.tsx | 104 ++++++++++++++- 5 files changed, 403 insertions(+), 15 deletions(-) diff --git a/docs/blazor-route-parity.md b/docs/blazor-route-parity.md index ed95b8e5e..046e2ea52 100644 --- a/docs/blazor-route-parity.md +++ b/docs/blazor-route-parity.md @@ -273,14 +273,20 @@ The removal PR is **gated** — it starts only after these clear: ~~#202 (media- **DONE 2026-07-11**, ~~#235 F9 (deep-scan + external-collections-scan API)~~ **API DONE (#235 slice B)**: `POST /api/libraries/{id}/scan?deep=` now threads deep-scan, and `POST /api/media-sources/{plex|jellyfin|emby}/{id}/scan-collections?deep=` covers external-collections scan (the two `Libraries.razor` parity gaps) — the SPA `Libraries.razor` port can now -proceed, and the **mandatory cold adversarial pass** (#91 comment 2026-07-09). **Remaining SPA affordance for -the removal PR** (API + thin `web/src/api/libraries.ts` clients — `scanLibrary(id, deep)`, `scanCollections` — -already shipped by #235): `LibrariesScreen` currently exposes only quick-scan; the removal PR must add the -**deep-scan** and **external-collections-scan** buttons (wiring the shipped clients) before deleting -`Libraries.razor`, or that capability is lost. (#204's id-carrying pattern -redirects landed 2026-07-11; its catch-all fallback that replaces `MapFallbackToPage("/_Host")` is folded -into Step 2 below, since it can only ship when `_Host` is deleted.) When those close, the removal-PR routine -executes, in order: +proceed, and the **mandatory cold adversarial pass** (#91 comment 2026-07-09). ~~**Remaining SPA affordance for +the removal PR**: `LibrariesScreen` exposes only quick-scan; add the **deep-scan** and **external-collections-scan** +buttons before deleting `Libraries.razor`.~~ **SPA affordance DONE 2026-07-11** (landed ahead of the removal PR +so the deletion diff stays pure): `LibrariesScreen` now wires the shipped `scanLibrary(id, deep)` + +`scanCollections(family, id, deep)` clients — a **Deep Scan Library** button on each remote library row and an +**External Collections** section (quick + deep per remote source), matching all four `Libraries.razor` scan actions. +The External Collections rows derive client-side from `getMediaSources()` (no new endpoint): the media-sources API +handler already filters each source's `libraries` to sync-enabled entries, so a remote source with a non-empty +`libraries` list is exactly `GetExternalCollections`'s `Libraries.Any(ShouldSyncItems)` filter. Collections scans +have no scan-status poll surface, so their button pending state is optimistic + timeout-bounded (follow-up #271: a +collections scan-status endpoint would let it reconcile like library scans). (#204's id-carrying +pattern redirects landed 2026-07-11; its catch-all fallback that replaces `MapFallbackToPage("/_Host")` is folded +into Step 2 below, since it can only ship when `_Host` is deleted.) With the SPA parity done, the removal PR's +remaining pre-work is just the **mandatory cold adversarial pass**; the runbook then executes, in order: 1. **Cut the rollback tag `blazor-final`** on the pre-deletion `main` commit — the *first* action, before deleting anything. Exact command + restore path: `docs/decisions.md` 2026-07-11 "Pre-removal Blazor diff --git a/docs/decisions.md b/docs/decisions.md index b533357d1..bffbafa87 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -777,3 +777,29 @@ the controller pre-guard) so a direct caller can't trip the `Max()`-on-empty cra the versioned aggregates — several of which this sweep also touched (post-commit token, a different line region). Low git-conflict risk, but merge `main` in and expect to see the `CancellationToken.None` convention already present on the post-commit enqueues. + +## 2026-07-11 — External Collections in the SPA derives from `getMediaSources()`; collections scans are optimistically bounded (#91b Libraries.razor parity) + +**Context.** The last SPA pre-work before deleting Blazor `Libraries.razor` was wiring the shipped +`scanLibrary(id, deep)` + `scanCollections(family, id, deep)` clients (F9 API, #235) into `LibrariesScreen`, +matching the page's four scan actions: quick-library (already had it), deep-library, quick-collections, +deep-collections. Landed as its own pure-SPA PR ahead of the removal PR so the deletion diff stays surgical. + +**Decision 1 — derive the External Collections rows client-side, no new endpoint.** Blazor's second table +comes from the `GetExternalCollections` MediatR query (one row per Plex/Jellyfin/Emby source with +`Libraries.Any(ShouldSyncItems)`), which has no REST wrapper. Rather than add one, the SPA derives the rows +from the existing `GET /api/media-sources`: `GetAllMediaSourcesForApiHandler` already filters each source's +`libraries` to sync-enabled entries (`ShouldIncludeLibrary` = `ShouldSyncItems` for remote families), so a +remote source with a **non-empty `libraries` list** is *exactly* the `Libraries.Any(ShouldSyncItems)` filter. +No extra endpoint, no extra fetch, single source of truth. (If `MediaSourceLibraryResponseModel` ever stops +filtering to sync-enabled libraries, this equivalence breaks — a test asserting the row set guards it.) + +**Decision 2 — collections-scan buttons use optimistic, timeout-bounded pending (no poll reconcile).** Library +scans reconcile their optimistic "pending" flag against `GET /api/libraries/scan-status`. Collections scans +have **no** such surface — that endpoint is library-keyed, and Blazor only ever observed collections locks via +in-process `IEntityLocker` `Are{X}CollectionsLocked` events with no HTTP mirror. So the `useCollectionsScan` +hook disables a button optimistically on click and clears it after a fixed `COLLECTIONS_PENDING_TIMEOUT_MS` +(a 409 "already scanning" is benign and keeps it disabled until the timeout; a 404/network clears it and +surfaces the error). This is the honest ceiling of what the current API exposes. **Follow-up (backlog):** a +collections scan-status endpoint would let the SPA reconcile collections pending against a live active set the +way library scans do; filed as **#271** so the timeout isn't mistaken for the intended end state. diff --git a/web/src/api/libraries.ts b/web/src/api/libraries.ts index 97b66ad98..c9263f8ad 100644 --- a/web/src/api/libraries.ts +++ b/web/src/api/libraries.ts @@ -16,7 +16,7 @@ export type LibrariesScreenQueryState = data: LibrariesScreenData; error: string | null; refresh: () => void; - scanLibrary: (libraryId: number) => Promise; + scanLibrary: (libraryId: number, deep?: boolean) => Promise; scanningLibraryIds: Set; status: 'success'; } @@ -315,9 +315,10 @@ export function useLibrariesScreenQuery(pollMs = 10000): LibrariesScreenQuerySta }); }, []); - const triggerScan = useCallback((libraryId: number): Promise => { + const triggerScan = useCallback((libraryId: number, deep = false): Promise => { if (pendingIdsRef.current.has(libraryId) || activeIdsRef.current.has(libraryId)) { - // Already pending or active - ignore the duplicate submission. + // Already pending or active - ignore the duplicate submission. Quick and deep scans share the + // same per-library lock, so both affordances gate on (and reconcile through) this one id set. return Promise.resolve(); } @@ -338,7 +339,7 @@ export function useLibrariesScreenQuery(pollMs = 10000): LibrariesScreenQuerySta }; }); - return scanLibrary(libraryId) + return scanLibrary(libraryId, deep) .then(() => { // 202 Accepted - a scan is genuinely queued. Poll scan-status; the pending flag is // promoted to "active" once the scan appears there. @@ -399,6 +400,107 @@ export function useLibrariesScreenQuery(pollMs = 10000): LibrariesScreenQuerySta return { data: null, error: null, refresh, status: 'loading' }; } +// --- External collections scan (Plex/Jellyfin/Emby) --- + +// A pending collections scan re-enables its button after this bound. Unlike library scans, an +// external-collections scan has NO authoritative "in progress" REST surface: /api/libraries/scan-status +// is library-keyed, and Blazor only ever observed collections locks through in-process IEntityLocker +// events (Are{X}CollectionsLocked) that have no HTTP mirror. So we can't reconcile "pending" against a +// live active set the way library scans do - we optimistically disable, then give up after this bound +// so a button can't wedge disabled forever. (A collections scan-status endpoint would let us reconcile +// properly - see the #91b follow-up note.) +const COLLECTIONS_PENDING_TIMEOUT_MS = 30000; + +export interface CollectionsScanState { + error: string | null; + scan: (source: CollectionsScanSource, sourceId: number, deep: boolean) => Promise; + scanningKeys: Set; +} + +// Stable key for a per-(family, source) collections scan, used by both the hook and the screen so the +// "is this row scanning?" lookup has exactly one definition. +export function collectionsScanKey(source: CollectionsScanSource, sourceId: number): string { + return `${source}:${sourceId}`; +} + +export function useCollectionsScan(): CollectionsScanState { + const [scanningKeys, setScanningKeys] = useState>(new Set()); + const [error, setError] = useState(null); + // Mirror kept in sync synchronously so scan() can guard double-submits without waiting for a render. + const scanningKeysRef = useRef>(new Set()); + const timeoutsRef = useRef>(new Map()); + const activeRef = useRef(true); + + useEffect(() => { + activeRef.current = true; + // Identity is stable across the hook's life (we only .set/.delete entries, never reassign the + // Map), so capturing it here is the same instance the cleanup clears at unmount. + const timeouts = timeoutsRef.current; + + return () => { + activeRef.current = false; + timeouts.forEach((timeoutId) => window.clearTimeout(timeoutId)); + timeouts.clear(); + }; + }, []); + + const clearKey = useCallback((key: string) => { + const timeoutId = timeoutsRef.current.get(key); + if (timeoutId !== undefined) { + window.clearTimeout(timeoutId); + timeoutsRef.current.delete(key); + } + + const next = new Set(scanningKeysRef.current); + next.delete(key); + scanningKeysRef.current = next; + + if (activeRef.current) { + setScanningKeys(next); + } + }, []); + + const scan = useCallback( + (source: CollectionsScanSource, sourceId: number, deep: boolean): Promise => { + const key = collectionsScanKey(source, sourceId); + if (scanningKeysRef.current.has(key)) { + // Already pending - ignore the duplicate submission (quick and deep share the source lock). + return Promise.resolve(); + } + + const pending = new Set(scanningKeysRef.current).add(key); + scanningKeysRef.current = pending; + setScanningKeys(pending); + setError(null); + + const timeoutId = window.setTimeout(() => clearKey(key), COLLECTIONS_PENDING_TIMEOUT_MS); + timeoutsRef.current.set(key, timeoutId); + + return scanCollections(source, sourceId, deep) + .then(() => { + // 202 Accepted - a scan is genuinely queued. Keep the button disabled until the bounded + // timeout expires (there is no completion signal to reconcile against). + }) + .catch((err: unknown) => { + if (err instanceof ApiError && err.status === 409) { + // 409 Conflict - a collections scan is already running for this source. Benign: keep the + // optimistic pending flag (button stays disabled) with no error; it clears on the timeout. + return; + } + + // 404 (source missing) / network error - nothing was queued, so re-enable and surface it. + clearKey(key); + if (activeRef.current) { + setError(messageFromLibrariesError(err, 'Unable to scan collections')); + } + }); + }, + [clearKey] + ); + + return { error, scan, scanningKeys }; +} + function messageFromLibrariesError(error: unknown, fallback = 'Unable to load libraries'): string { if (error instanceof ApiError) { return error.detail ?? error.message; diff --git a/web/src/screens/LibrariesScreen.test.tsx b/web/src/screens/LibrariesScreen.test.tsx index 8eedd24d3..9d17b698e 100644 --- a/web/src/screens/LibrariesScreen.test.tsx +++ b/web/src/screens/LibrariesScreen.test.tsx @@ -74,7 +74,14 @@ function mockApi(options: MockOptions = {}): MockHandle { return Promise.resolve(jsonResponse(options.libraryScanStatuses ?? [])); } - if (/^\/api\/libraries\/\d+\/scan$/.test(url) && method === 'POST') { + if (/^\/api\/libraries\/\d+\/scan(\?deep=true)?$/.test(url) && method === 'POST') { + if (url in mutationFailures) { + return Promise.resolve(jsonResponse(mutationFailures[url], mutationFailures[url].status ?? 422)); + } + return Promise.resolve(new Response(null, { status: 202 })); + } + + if (/^\/api\/media-sources\/(plex|jellyfin|emby)\/\d+\/scan-collections(\?deep=true)?$/.test(url) && method === 'POST') { if (url in mutationFailures) { return Promise.resolve(jsonResponse(mutationFailures[url], mutationFailures[url].status ?? 422)); } @@ -369,6 +376,153 @@ describe('LibrariesScreen', () => { expect(await screen.findByText('No media sources returned')).toBeInTheDocument(); }); + describe('deep scan + external collections (#91b Libraries.razor parity)', () => { + // A remote source with two sync-enabled libraries (deep-scan available) plus an External + // Collections row; a Local source (no deep, no collections row); and a remote source with NO + // sync-enabled libraries (no External Collections row). + function paritySources(): unknown[] { + return [ + mediaSource({ + id: 30, + kind: 'Local', + libraries: [library({ id: 31, mediaKind: 'Movies', name: 'Movies' })], + name: 'Local' + }), + mediaSource({ + connectionAddress: 'https://plex.example.test', + id: 40, + kind: 'Plex', + libraries: [library({ id: 41, mediaKind: 'Shows', name: 'TV Shows' })], + name: 'Plex Server' + }), + mediaSource({ + connectionAddress: 'https://jellyfin.example.test', + id: 50, + kind: 'Jellyfin', + libraries: [], + name: 'Jellyfin Home' + }) + ]; + } + + it('deep-scans a remote library via POST /api/libraries/{id}/scan?deep=true', async () => { + const handle = mockApi({ mediaSources: paritySources() }); + + render(); + await screen.findByText('TV Shows'); + + fireEvent.click(screen.getByRole('button', { name: 'Deep scan TV Shows' })); + + await waitFor(() => { + expect(handle.fetchSpy).toHaveBeenCalledWith('/api/libraries/41/scan?deep=true', expect.objectContaining({ method: 'POST' })); + }); + expect(fetchCount(handle, '/api/libraries/41/scan')).toBe(0); + }); + + it('offers a deep-scan button only for remote libraries, never Local', async () => { + mockApi({ mediaSources: paritySources() }); + + render(); + await screen.findByText('TV Shows'); + + // Local library "Movies" has a quick scan but no deep scan. + expect(screen.getByRole('button', { name: 'Scan Movies' })).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: 'Deep scan Movies' })).not.toBeInTheDocument(); + // Remote library "TV Shows" has both. + expect(screen.getByRole('button', { name: 'Scan TV Shows' })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Deep scan TV Shows' })).toBeInTheDocument(); + }); + + it('renders an External Collections row only for remote sources with sync-enabled libraries', async () => { + mockApi({ mediaSources: paritySources() }); + + render(); + await screen.findByText('TV Shows'); + + expect(screen.getByRole('heading', { name: 'External Collections' })).toBeInTheDocument(); + // Plex Server has a sync-enabled library → a collections row. + expect(screen.getByRole('button', { name: 'Scan Plex Server collections' })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Deep scan Plex Server collections' })).toBeInTheDocument(); + // Jellyfin Home has no sync-enabled library → no collections row; Local never gets one. + expect(screen.queryByRole('button', { name: 'Scan Jellyfin Home collections' })).not.toBeInTheDocument(); + expect(screen.queryByRole('button', { name: 'Scan Local collections' })).not.toBeInTheDocument(); + }); + + it('omits the External Collections section entirely when no remote source has sync-enabled libraries', async () => { + mockApi({ + mediaSources: [ + mediaSource({ id: 30, kind: 'Local', libraries: [library({ id: 31, name: 'Movies' })], name: 'Local' }) + ] + }); + + render(); + expect((await screen.findAllByText('Movies')).length).toBeGreaterThan(0); + + expect(screen.queryByRole('heading', { name: 'External Collections' })).not.toBeInTheDocument(); + }); + + it('queues a quick collections scan via POST /api/media-sources/{family}/{id}/scan-collections', async () => { + const handle = mockApi({ mediaSources: paritySources() }); + + render(); + await screen.findByRole('button', { name: 'Scan Plex Server collections' }); + + fireEvent.click(screen.getByRole('button', { name: 'Scan Plex Server collections' })); + + await waitFor(() => { + expect(handle.fetchSpy).toHaveBeenCalledWith('/api/media-sources/plex/40/scan-collections', expect.objectContaining({ method: 'POST' })); + }); + // The button is optimistically disabled while the scan is pending. + expect(screen.getByRole('button', { name: 'Scan Plex Server collections' })).toBeDisabled(); + }); + + it('queues a deep collections scan with ?deep=true', async () => { + const handle = mockApi({ mediaSources: paritySources() }); + + render(); + await screen.findByRole('button', { name: 'Deep scan Plex Server collections' }); + + fireEvent.click(screen.getByRole('button', { name: 'Deep scan Plex Server collections' })); + + await waitFor(() => { + expect(handle.fetchSpy).toHaveBeenCalledWith('/api/media-sources/plex/40/scan-collections?deep=true', expect.objectContaining({ method: 'POST' })); + }); + }); + + it('treats a 409 "already scanning" collections response as benign (button stays disabled, no error)', async () => { + mockApi({ + mediaSources: paritySources(), + mutationFailures: { '/api/media-sources/plex/40/scan-collections': { status: 409 } } + }); + + render(); + await screen.findByRole('button', { name: 'Scan Plex Server collections' }); + + fireEvent.click(screen.getByRole('button', { name: 'Scan Plex Server collections' })); + + await waitFor(() => { + expect(screen.getByRole('button', { name: 'Scan Plex Server collections' })).toBeDisabled(); + }); + expect(screen.queryByText(/Unable to scan collections/)).not.toBeInTheDocument(); + expect(screen.queryByText(/Request failed/)).not.toBeInTheDocument(); + }); + + it('surfaces a collections scan failure and re-enables the button', async () => { + mockApi({ + mediaSources: paritySources(), + mutationFailures: { '/api/media-sources/plex/40/scan-collections': { status: 404 } } + }); + + render(); + await screen.findByRole('button', { name: 'Scan Plex Server collections' }); + + fireEvent.click(screen.getByRole('button', { name: 'Scan Plex Server collections' })); + + expect(await screen.findByText('Request failed with status 404')).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Scan Plex Server collections' })).not.toBeDisabled(); + }); + }); + describe('hub wiring (slice S5)', () => { it('Add Source menu navigates to the local-new and each family route', async () => { const pushSpy = vi.spyOn(window.history, 'pushState'); diff --git a/web/src/screens/LibrariesScreen.tsx b/web/src/screens/LibrariesScreen.tsx index 8bec33cae..3ae7ce712 100644 --- a/web/src/screens/LibrariesScreen.tsx +++ b/web/src/screens/LibrariesScreen.tsx @@ -6,11 +6,13 @@ import { Film, Folder, HardDrive, + Library, MonitorPlay, Music, Plus, Radio, RefreshCw, + ScanSearch, Server, Settings, TriangleAlert @@ -25,7 +27,10 @@ import { StatusDot } from '../components'; import { + collectionsScanKey, + useCollectionsScan, useLibrariesScreenQuery, + type CollectionsScanSource, type LibraryScanStatus, type MediaSource, type MediaSourceLibrary @@ -41,6 +46,7 @@ import { export function LibrariesScreen() { const query = useLibrariesScreenQuery(); + const collectionsScan = useCollectionsScan(); if (query.status === 'loading') { return ; @@ -60,6 +66,21 @@ export function LibrariesScreen() { ); const scanStatusesByLibraryId = new Map(query.data.scanStatuses.map((status) => [status.libraryId, status])); + // External Collections mirrors Blazor Libraries.razor's second table: one row per Plex/Jellyfin/Emby + // source that has at least one sync-enabled library. GetAllMediaSourcesForApiHandler already filters + // each source's `libraries` to sync-enabled entries (ShouldIncludeLibrary), so a remote source with a + // non-empty `libraries` list is exactly the `Libraries.Any(ShouldSyncItems)` filter GetExternalCollections + // applies - no separate endpoint needed. + const collectionsRows = query.data.sources + .map((source) => { + const family = parseRemoteFamily(source.kind); + if (family === null || source.libraries.length === 0) { + return null; + } + return { family, name: source.name, sourceId: source.id }; + }) + .filter((row): row is { family: CollectionsScanSource; name: string; sourceId: number } => row !== null); + return (
@@ -97,10 +118,81 @@ export function LibrariesScreen() { /> ))}
+ + {collectionsRows.length > 0 && ( + + )} ); } +// External Collections table (parity with Blazor Libraries.razor's second table). Each row is a +// remote source with sync-enabled libraries; the quick/deep buttons queue that source's collections +// scan. There is no scan-status poll surface for collections (see useCollectionsScan), so a row's +// "scanning" state is optimistic and bounded by a timeout rather than reconciled against a live set. +function ExternalCollectionsSection({ + error, + onScanCollections, + rows, + scanningKeys +}: { + error: string | null; + onScanCollections: (source: CollectionsScanSource, sourceId: number, deep: boolean) => Promise; + rows: Array<{ family: CollectionsScanSource; name: string; sourceId: number }>; + scanningKeys: Set; +}) { + return ( +
+
+ +
+
+

External Collections

+
+ Sync Plex, Jellyfin, and Emby collections into ErsatzTV. +
+
+ + {error && ( +
+
+ )} + +
+ {rows.map((row) => { + const scanning = scanningKeys.has(collectionsScanKey(row.family, row.sourceId)); + + return ( +
+ +
+ {row.name} + {familyLabel(row.family)} collections +
+
+ {scanning ? Scanning : Idle} + void onScanCollections(row.family, row.sourceId, true)} size="sm" title={`Deep scan ${row.name} collections`}> + + void onScanCollections(row.family, row.sourceId, false)} size="sm" title={`Scan ${row.name} collections`}> + +
+
+ ); + })} +
+
+ ); +} + // "Add Source" dropdown: Local library opens the create editor; each remote family opens its screen. // Mirrors the AddToMenu popover pattern (click-outside + Escape close). Navigation goes through // navigateToPath, which App's popstate owner turns into the right libraries sub-path (design §D.2). @@ -203,7 +295,7 @@ function MediaSourceCard({ scanningLibraryIds, source }: { - onScanLibrary: (libraryId: number) => Promise; + onScanLibrary: (libraryId: number, deep?: boolean) => Promise; scanStatusesByLibraryId: Map; scanningLibraryIds: Set; source: MediaSource; @@ -250,6 +342,7 @@ function MediaSourceCard({ ) : source.libraries.map((library) => ( Promise; + onScanLibrary: (libraryId: number, deep?: boolean) => Promise; scanStatus: LibraryScanStatus | null; scanning: boolean; }) { @@ -300,6 +395,11 @@ function LibraryRow({ ) : ( Synced )} + {allowDeepScan && ( + void onScanLibrary(library.id, true)} size="sm" title={`Deep scan ${library.name}`}> + + )} void onScanLibrary(library.id)} size="sm" title={`Scan ${library.name}`}>