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/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 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 eceef1ecd..715e14cfa 100644 --- a/docs/api-conventions.md +++ b/docs/api-conventions.md @@ -343,6 +343,38 @@ save → 412 (prove it non-vacuous by dropping `.IsConcurrencyToken()` and watch Phase 2 (a later PR) flips a missing `If-Match` from force-write to **428 Precondition Required** once every editor echoes and one release soaks. +## 7b. 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 40513f80d..e980eb8c7 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -682,3 +682,44 @@ the mechanics live in `api-conventions.md` §7a. Decisions frozen here: concern on the positional reconcile) — root-anchored versioning is orthogonal to it; split to **#259**. - **If-Match status semantics** (non-canonical/weak/list → 400) are fail-safe; the stricter RFC 7232 "valid-but-non-matching → 412" refinement is deferred to #197 (**#265**). + +## 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` §7b. + +**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.