From 28910ff557501d62e279c1f05b0dd701a23bc367 Mon Sep 17 00:00:00 2001 From: Timothy Date: Fri, 10 Jul 2026 23:34:23 +0200 Subject: [PATCH] fix(api): gate playout mutations on EntityLocker build lock (409) + mirror lock state in SPA (fixes #215) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Blazor disabled per-playout Reset/Erase/Delete/Edit while a BuildPlayout was in flight (EntityLocker.IsPlayoutLocked); the REST API had no equivalent, so a client could race an in-flight build with a destructive ExecuteDelete and leave a half-built playout. After Blazor removal this safety invariant would vanish entirely (adversarial-reviewer#18 removal gate). Server: - Add public ApiResults.ConflictProblem(title, detail) (409, mirrors NotFoundProblem). - Inject IEntityLocker into PlayoutController; guard every id-keyed mutation (PUT {id}, PUT .../deco, PUT .../alternate-schedules, PUT .../templates, POST .../erase-items, POST .../erase-items-and-history, DELETE {id}) → 409 when IsPlayoutLocked(id); add [ProducesResponseType(...409)] to each. - Guard ChannelController.ResetPlayout the same way after resolving the id. - reset-all stays 202 (ResetAllPlayoutsHandler already skips locked playouts). - Stamp IsLocked onto PlayoutListItemResponseModel from IsPlayoutLocked. SPA: - Disable Reset/Erase/Erase-and-history/Delete for a locked row + show a "Building…" Badge; on a 409 surface the error and refresh the list. Tests: controller-level 409 guard tests (delete/erase/PUT/deco/channel-reset) + IsLocked projection test; new OpenAPI contract + metadata 409 rows. Docs: api-conventions §3a, blazor-route-parity playouts verdict, decisions.md. Regenerated v1.json + web types. Co-Authored-By: Claude Fable 5 --- .../Playouts/PlayoutListItemResponseModel.cs | 3 +- .../ApiErrorResponseMetadataTests.cs | 12 ++ .../Controllers/ChannelControllerTests.cs | 19 +- .../OpenApiErrorResponseContractTests.cs | 10 ++ .../Controllers/PlayoutControllerTests.cs | 81 ++++++++- ErsatzTV/Controllers/Api/ChannelController.cs | 15 +- ErsatzTV/Controllers/Api/PlayoutController.cs | 66 ++++++- ErsatzTV/Extensions/ApiResults.cs | 7 + ErsatzTV/wwwroot/openapi/v1.json | 166 +++++++++++++++++- docs/api-conventions.md | 21 +++ docs/blazor-route-parity.md | 8 +- docs/decisions.md | 29 +++ web/src/App.test.tsx | 20 +++ web/src/App.tsx | 23 ++- web/src/api/generated/v1.d.ts | 1 + 15 files changed, 466 insertions(+), 15 deletions(-) diff --git a/ErsatzTV.Core/Api/Playouts/PlayoutListItemResponseModel.cs b/ErsatzTV.Core/Api/Playouts/PlayoutListItemResponseModel.cs index 33aa15d5b..a5da52036 100644 --- a/ErsatzTV.Core/Api/Playouts/PlayoutListItemResponseModel.cs +++ b/ErsatzTV.Core/Api/Playouts/PlayoutListItemResponseModel.cs @@ -11,4 +11,5 @@ public record PlayoutListItemResponseModel( string ScheduleName, TimeSpan? DailyRebuildTime, PlayoutBuildStatusResponseModel? BuildStatus, - ChannelPlayoutMode PlayoutMode); + ChannelPlayoutMode PlayoutMode, + bool IsLocked); diff --git a/ErsatzTV.Tests/Controllers/ApiErrorResponseMetadataTests.cs b/ErsatzTV.Tests/Controllers/ApiErrorResponseMetadataTests.cs index 7ab30a490..f4833f19a 100644 --- a/ErsatzTV.Tests/Controllers/ApiErrorResponseMetadataTests.cs +++ b/ErsatzTV.Tests/Controllers/ApiErrorResponseMetadataTests.cs @@ -26,6 +26,7 @@ public class ApiErrorResponseMetadataTests [TestCase(typeof(ChannelController), nameof(ChannelController.BulkDelete), StatusCodes.Status404NotFound)] [TestCase(typeof(ChannelController), nameof(ChannelController.BulkDelete), StatusCodes.Status422UnprocessableEntity)] [TestCase(typeof(ChannelController), nameof(ChannelController.ResetPlayout), StatusCodes.Status404NotFound)] + [TestCase(typeof(ChannelController), nameof(ChannelController.ResetPlayout), StatusCodes.Status409Conflict)] [TestCase(typeof(ChannelTemplateController), nameof(ChannelTemplateController.GetDefault), StatusCodes.Status404NotFound)] [TestCase(typeof(ChannelTemplateController), nameof(ChannelTemplateController.SetDefault), StatusCodes.Status404NotFound)] [TestCase(typeof(ChannelTemplateController), nameof(ChannelTemplateController.SetDefault), StatusCodes.Status422UnprocessableEntity)] @@ -113,22 +114,33 @@ public class ApiErrorResponseMetadataTests [TestCase(typeof(DecoTemplateController), nameof(DecoTemplateController.Replace), StatusCodes.Status404NotFound)] [TestCase(typeof(DecoTemplateController), nameof(DecoTemplateController.Replace), StatusCodes.Status422UnprocessableEntity)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.UpdateDefaultDeco), StatusCodes.Status404NotFound)] + [TestCase(typeof(PlayoutController), nameof(PlayoutController.UpdateDefaultDeco), StatusCodes.Status409Conflict)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.UpdateDefaultDeco), StatusCodes.Status422UnprocessableEntity)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.GetById), StatusCodes.Status404NotFound)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.Create), StatusCodes.Status404NotFound)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.Create), StatusCodes.Status422UnprocessableEntity)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.Update), StatusCodes.Status404NotFound)] + [TestCase(typeof(PlayoutController), nameof(PlayoutController.Update), StatusCodes.Status409Conflict)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.Update), StatusCodes.Status422UnprocessableEntity)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.Delete), StatusCodes.Status404NotFound)] + [TestCase(typeof(PlayoutController), nameof(PlayoutController.Delete), StatusCodes.Status409Conflict)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.Delete), StatusCodes.Status422UnprocessableEntity)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.GetAlternateSchedules), StatusCodes.Status404NotFound)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.GetAlternateSchedules), StatusCodes.Status422UnprocessableEntity)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.ReplaceAlternateSchedules), StatusCodes.Status404NotFound)] + [TestCase(typeof(PlayoutController), nameof(PlayoutController.ReplaceAlternateSchedules), StatusCodes.Status409Conflict)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.ReplaceAlternateSchedules), StatusCodes.Status422UnprocessableEntity)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.GetTemplates), StatusCodes.Status404NotFound)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.GetTemplates), StatusCodes.Status422UnprocessableEntity)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.ReplaceTemplates), StatusCodes.Status404NotFound)] + [TestCase(typeof(PlayoutController), nameof(PlayoutController.ReplaceTemplates), StatusCodes.Status409Conflict)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.ReplaceTemplates), StatusCodes.Status422UnprocessableEntity)] + [TestCase(typeof(PlayoutController), nameof(PlayoutController.EraseItems), StatusCodes.Status404NotFound)] + [TestCase(typeof(PlayoutController), nameof(PlayoutController.EraseItems), StatusCodes.Status409Conflict)] + [TestCase(typeof(PlayoutController), nameof(PlayoutController.EraseItems), StatusCodes.Status422UnprocessableEntity)] + [TestCase(typeof(PlayoutController), nameof(PlayoutController.EraseItemsAndHistory), StatusCodes.Status404NotFound)] + [TestCase(typeof(PlayoutController), nameof(PlayoutController.EraseItemsAndHistory), StatusCodes.Status409Conflict)] + [TestCase(typeof(PlayoutController), nameof(PlayoutController.EraseItemsAndHistory), StatusCodes.Status422UnprocessableEntity)] [TestCase(typeof(FFmpegProfileController), nameof(FFmpegProfileController.GetById), StatusCodes.Status404NotFound)] [TestCase(typeof(FFmpegProfileController), nameof(FFmpegProfileController.AddOne), StatusCodes.Status404NotFound)] [TestCase(typeof(FFmpegProfileController), nameof(FFmpegProfileController.AddOne), StatusCodes.Status401Unauthorized)] diff --git a/ErsatzTV.Tests/Controllers/ChannelControllerTests.cs b/ErsatzTV.Tests/Controllers/ChannelControllerTests.cs index 9377d2f7c..cd25e7760 100644 --- a/ErsatzTV.Tests/Controllers/ChannelControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/ChannelControllerTests.cs @@ -10,6 +10,7 @@ using ErsatzTV.Core.Api.Channels; using ErsatzTV.Core.Api.LibraryBrowse; using ErsatzTV.Core.Domain; using ErsatzTV.Core.Errors; +using ErsatzTV.Core.Interfaces.Locking; using ErsatzTV.Core.Scheduling; using LanguageExt; using static LanguageExt.Prelude; @@ -27,6 +28,7 @@ public class ChannelControllerTests { private IMediator _mediator = null!; private Channel _workerChannel = null!; + private IEntityLocker _entityLocker = null!; private ChannelController _controller = null!; [SetUp] @@ -34,7 +36,8 @@ public class ChannelControllerTests { _mediator = Substitute.For(); _workerChannel = System.Threading.Channels.Channel.CreateUnbounded(); - _controller = new ChannelController(_workerChannel.Writer, _mediator); + _entityLocker = Substitute.For(); + _controller = new ChannelController(_workerChannel.Writer, _mediator, _entityLocker); } [Test] @@ -374,6 +377,20 @@ public class ChannelControllerTests buildPlayout.Mode.ShouldBe(expectedMode); } + [Test] + public async Task ResetPlayout_Should_Return_409_When_Playout_Locked() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(9)); + _entityLocker.IsPlayoutLocked(9).Returns(true); + + IActionResult result = await _controller.ResetPlayout("5", mode: null, CancellationToken.None); + + var conflict = result.ShouldBeOfType(); + conflict.Value.ShouldBeOfType().Status.ShouldBe(409); + _workerChannel.Reader.TryRead(out _).ShouldBeFalse(); + } + [Test] public async Task ResetPlayout_Should_Honor_Explicit_Mode() { diff --git a/ErsatzTV.Tests/Controllers/OpenApiErrorResponseContractTests.cs b/ErsatzTV.Tests/Controllers/OpenApiErrorResponseContractTests.cs index a24049ea0..48eee50c5 100644 --- a/ErsatzTV.Tests/Controllers/OpenApiErrorResponseContractTests.cs +++ b/ErsatzTV.Tests/Controllers/OpenApiErrorResponseContractTests.cs @@ -115,6 +115,7 @@ public class OpenApiErrorResponseContractTests [TestCase("/api/channels/bulk/delete", "post", "404")] [TestCase("/api/channels/bulk/delete", "post", "422")] [TestCase("/api/channels/{channelNumber}/playout/reset", "post", "404")] + [TestCase("/api/channels/{channelNumber}/playout/reset", "post", "409")] [TestCase("/api/channel-templates/default", "get", "404")] [TestCase("/api/channel-templates/default/{id}", "put", "404")] [TestCase("/api/channel-templates/default/{id}", "put", "422")] @@ -188,12 +189,21 @@ public class OpenApiErrorResponseContractTests [TestCase("/api/playouts/{id}", "get", "404")] [TestCase("/api/playouts", "post", "404")] [TestCase("/api/playouts", "post", "422")] + [TestCase("/api/playouts/{id}", "put", "404")] + [TestCase("/api/playouts/{id}", "put", "409")] + [TestCase("/api/playouts/{id}", "put", "422")] [TestCase("/api/playouts/{id}", "delete", "404")] + [TestCase("/api/playouts/{id}", "delete", "409")] [TestCase("/api/playouts/{id}", "delete", "422")] + [TestCase("/api/playouts/{id}/deco", "put", "409")] + [TestCase("/api/playouts/{id}/alternate-schedules", "put", "409")] + [TestCase("/api/playouts/{id}/templates", "put", "409")] [TestCase("/api/playouts/{id}/items", "get", "404")] [TestCase("/api/playouts/{id}/erase-items", "post", "404")] + [TestCase("/api/playouts/{id}/erase-items", "post", "409")] [TestCase("/api/playouts/{id}/erase-items", "post", "422")] [TestCase("/api/playouts/{id}/erase-items-and-history", "post", "404")] + [TestCase("/api/playouts/{id}/erase-items-and-history", "post", "409")] [TestCase("/api/playouts/{id}/erase-items-and-history", "post", "422")] [TestCase("/api/playouts/items/{id}/scheduling-context", "get", "404")] [TestCase("/api/collections/{id}/custom-order", "put", "404")] diff --git a/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs b/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs index cd285aad6..2f8a85930 100644 --- a/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs @@ -13,6 +13,7 @@ using ErsatzTV.Core.Domain; using ErsatzTV.Core.Domain.Filler; using ErsatzTV.Core.Domain.Scheduling; using ErsatzTV.Core.Errors; +using ErsatzTV.Core.Interfaces.Locking; using ErsatzTV.Core.Scheduling; using LanguageExt; using MediatR; @@ -31,12 +32,14 @@ public class PlayoutControllerTests { private PlayoutController _controller = null!; private IMediator _mediator = null!; + private IEntityLocker _entityLocker = null!; [SetUp] public void SetUp() { _mediator = Substitute.For(); - _controller = new PlayoutController(_mediator); + _entityLocker = Substitute.For(); + _controller = new PlayoutController(_mediator, _entityLocker); } [Test] @@ -68,6 +71,82 @@ public class PlayoutControllerTests "/api/playouts/items/{id:int}/scheduling-context"); } + // ----- Build-lock guard (#215): id-keyed mutations return 409 while the build lock is held ----- + + [Test] + public async Task Delete_Should_Return_409_When_Playout_Locked() + { + _entityLocker.IsPlayoutLocked(9).Returns(true); + + IActionResult result = await _controller.Delete(9, CancellationToken.None); + + var conflict = result.ShouldBeOfType(); + conflict.Value.ShouldBeOfType().Status.ShouldBe(409); + await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + } + + [Test] + public async Task EraseItems_Should_Return_409_When_Playout_Locked() + { + _entityLocker.IsPlayoutLocked(9).Returns(true); + + IActionResult result = await _controller.EraseItems(9, CancellationToken.None); + + result.ShouldBeOfType().Value.ShouldBeOfType().Status.ShouldBe(409); + await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + } + + [Test] + public async Task EraseItemsAndHistory_Should_Return_409_When_Playout_Locked() + { + _entityLocker.IsPlayoutLocked(9).Returns(true); + + IActionResult result = await _controller.EraseItemsAndHistory(9, CancellationToken.None); + + result.ShouldBeOfType().Value.ShouldBeOfType().Status.ShouldBe(409); + await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + } + + [Test] + public async Task Update_Should_Return_409_When_Playout_Locked() + { + _entityLocker.IsPlayoutLocked(9).Returns(true); + + IActionResult result = await _controller.Update( + 9, + new UpdatePlayoutDetailsRequest(TimeSpan.FromHours(4), null), + CancellationToken.None); + + result.ShouldBeOfType().Value.ShouldBeOfType().Status.ShouldBe(409); + await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + } + + [Test] + public async Task UpdateDefaultDeco_Should_Return_409_When_Playout_Locked() + { + _entityLocker.IsPlayoutLocked(9).Returns(true); + + IActionResult result = await _controller.UpdateDefaultDeco( + 9, + new UpdateDefaultDecoRequest(null), + CancellationToken.None); + + result.ShouldBeOfType().Value.ShouldBeOfType().Status.ShouldBe(409); + } + + [Test] + public async Task GetAll_Should_Stamp_IsLocked_From_Locker() + { + _entityLocker.IsPlayoutLocked(9).Returns(true); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(new PagedPlayoutsViewModel(1, [MakePlayout(9)])); + + PagedPlayoutsResponseModel result = await _controller.GetAll("", 0, 100, CancellationToken.None); + + result.Page.Single().IsLocked.ShouldBeTrue(); + } + // ----- Erase items / history ----- [Test] diff --git a/ErsatzTV/Controllers/Api/ChannelController.cs b/ErsatzTV/Controllers/Api/ChannelController.cs index 9082d7f3b..e675e1ec2 100644 --- a/ErsatzTV/Controllers/Api/ChannelController.cs +++ b/ErsatzTV/Controllers/Api/ChannelController.cs @@ -7,6 +7,7 @@ using ErsatzTV.Controllers.Api.Requests; using ErsatzTV.Core; using ErsatzTV.Core.Api.Channels; using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Interfaces.Locking; using ErsatzTV.Core.Scheduling; using ErsatzTV.Extensions; using MediatR; @@ -16,7 +17,10 @@ using Microsoft.AspNetCore.Mvc; namespace ErsatzTV.Controllers.Api; [ApiController] -public class ChannelController(ChannelWriter workerChannel, IMediator mediator) +public class ChannelController( + ChannelWriter workerChannel, + IMediator mediator, + IEntityLocker entityLocker) { [HttpGet("/api/channels")] [EndpointGroupName("general")] @@ -192,6 +196,7 @@ public class ChannelController(ChannelWriter workerCh [EndpointGroupName("general")] [ProducesResponseType(StatusCodes.Status200OK)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] public async Task ResetPlayout( string channelNumber, [FromQuery] PlayoutBuildMode? mode, @@ -201,6 +206,14 @@ public class ChannelController(ChannelWriter workerCh await mediator.Send(new GetPlayoutIdByChannelNumber(channelNumber), cancellationToken); foreach (int playoutId in maybePlayoutId) { + // Mirror Blazor's EntityLocker gating: don't enqueue a rebuild while one is already in flight. + if (entityLocker.IsPlayoutLocked(playoutId)) + { + return ApiResults.ConflictProblem( + "Playout build in progress", + "A build for this playout is currently in progress; try again once it completes."); + } + PlayoutBuildMode buildMode = mode ?? await DefaultResetMode(playoutId, cancellationToken); await workerChannel.WriteAsync(new BuildPlayout(playoutId, buildMode), cancellationToken); return new OkResult(); diff --git a/ErsatzTV/Controllers/Api/PlayoutController.cs b/ErsatzTV/Controllers/Api/PlayoutController.cs index d7da83203..a0bff515f 100644 --- a/ErsatzTV/Controllers/Api/PlayoutController.cs +++ b/ErsatzTV/Controllers/Api/PlayoutController.cs @@ -10,6 +10,7 @@ using ErsatzTV.Core.Api.Playouts; using ErsatzTV.Core.Api.Scheduling; using ErsatzTV.Core.Domain; using ErsatzTV.Core.Domain.Filler; +using ErsatzTV.Core.Interfaces.Locking; using ErsatzTV.Extensions; using MediatR; using Microsoft.AspNetCore.Http; @@ -18,10 +19,21 @@ using Microsoft.AspNetCore.Mvc; namespace ErsatzTV.Controllers.Api; [ApiController] -public class PlayoutController(IMediator mediator) : ControllerBase +public class PlayoutController(IMediator mediator, IEntityLocker entityLocker) : ControllerBase { private const int MaxPageSize = 100; + // Blazor disables per-playout Reset/Erase/Delete/Edit while a build is in flight + // (EntityLocker.IsPlayoutLocked); the API mirrors that invariant by rejecting any + // id-keyed mutation with 409 while the build lock is held. See docs/decisions.md 2026-07-10. + private const string BuildInProgressTitle = "Playout build in progress"; + + private const string BuildInProgressDetail = + "A build for this playout is currently in progress; try again once it completes."; + + private static IActionResult PlayoutLockedProblem() => + ApiResults.ConflictProblem(BuildInProgressTitle, BuildInProgressDetail); + [HttpGet("/api/playouts", Name = "GetPlayouts")] [Tags("Playouts")] [EndpointSummary("List playouts")] @@ -37,7 +49,7 @@ public class PlayoutController(IMediator mediator) : ControllerBase await mediator.Send(new GetPagedPlayouts(query, pageNum, pageSize), cancellationToken); return new PagedPlayoutsResponseModel( result.TotalCount, - result.Page.Map(ToListItemResponse).ToList()); + result.Page.Map(vm => ToListItemResponse(vm, entityLocker.IsPlayoutLocked(vm.PlayoutId))).ToList()); } [HttpGet("/api/playouts/warnings/count", Name = "GetPlayoutWarningsCount")] @@ -131,12 +143,18 @@ public class PlayoutController(IMediator mediator) : ControllerBase [EndpointGroupName("general")] [ProducesResponseType(typeof(PlayoutResponseModel), StatusCodes.Status200OK)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task Update( int id, [Required] [FromBody] UpdatePlayoutDetailsRequest request, CancellationToken cancellationToken) { + if (entityLocker.IsPlayoutLocked(id)) + { + return PlayoutLockedProblem(); + } + Option maybePlayout = await mediator.Send(new GetPlayoutById(id), cancellationToken); if (maybePlayout.IsNone) { @@ -204,12 +222,18 @@ public class PlayoutController(IMediator mediator) : ControllerBase [EndpointGroupName("general")] [ProducesResponseType(typeof(PlayoutResponseModel), StatusCodes.Status200OK)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task UpdateDefaultDeco( int id, [Required] [FromBody] UpdateDefaultDecoRequest request, CancellationToken cancellationToken) { + if (entityLocker.IsPlayoutLocked(id)) + { + return PlayoutLockedProblem(); + } + Option maybePlayout = await mediator.Send(new GetPlayoutById(id), cancellationToken); if (maybePlayout.IsNone) { @@ -284,12 +308,18 @@ public class PlayoutController(IMediator mediator) : ControllerBase [EndpointGroupName("general")] [ProducesResponseType(typeof(List), StatusCodes.Status200OK)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task ReplaceAlternateSchedules( int id, [Required] [FromBody] ReplacePlayoutAlternateSchedulesRequest request, CancellationToken cancellationToken) { + if (entityLocker.IsPlayoutLocked(id)) + { + return PlayoutLockedProblem(); + } + Option maybePlayout = await mediator.Send(new GetPlayoutById(id), cancellationToken); if (maybePlayout.IsNone) { @@ -376,12 +406,18 @@ public class PlayoutController(IMediator mediator) : ControllerBase [EndpointGroupName("general")] [ProducesResponseType(typeof(List), StatusCodes.Status200OK)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task ReplaceTemplates( int id, [Required] [FromBody] ReplacePlayoutTemplatesRequest request, CancellationToken cancellationToken) { + if (entityLocker.IsPlayoutLocked(id)) + { + return PlayoutLockedProblem(); + } + Option maybePlayout = await mediator.Send(new GetPlayoutById(id), cancellationToken); if (maybePlayout.IsNone) { @@ -515,6 +551,9 @@ public class PlayoutController(IMediator mediator) : ControllerBase [EndpointSummary("Reset all playouts")] [EndpointGroupName("general")] [ProducesResponseType(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. public async Task ResetAll(CancellationToken cancellationToken) { await mediator.Send(new ResetAllPlayouts(), cancellationToken); @@ -531,9 +570,15 @@ public class PlayoutController(IMediator mediator) : ControllerBase [EndpointGroupName("general")] [ProducesResponseType(StatusCodes.Status204NoContent)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task EraseItems(int id, CancellationToken cancellationToken) { + if (entityLocker.IsPlayoutLocked(id)) + { + return PlayoutLockedProblem(); + } + Option maybePlayout = await mediator.Send(new GetPlayoutById(id), cancellationToken); if (maybePlayout.IsNone) { @@ -563,9 +608,15 @@ public class PlayoutController(IMediator mediator) : ControllerBase [EndpointGroupName("general")] [ProducesResponseType(StatusCodes.Status204NoContent)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task EraseItemsAndHistory(int id, CancellationToken cancellationToken) { + if (entityLocker.IsPlayoutLocked(id)) + { + return PlayoutLockedProblem(); + } + Option maybePlayout = await mediator.Send(new GetPlayoutById(id), cancellationToken); if (maybePlayout.IsNone) { @@ -608,9 +659,15 @@ public class PlayoutController(IMediator mediator) : ControllerBase [EndpointGroupName("general")] [ProducesResponseType(StatusCodes.Status204NoContent)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task Delete(int id, CancellationToken cancellationToken) { + if (entityLocker.IsPlayoutLocked(id)) + { + return PlayoutLockedProblem(); + } + Either result = await mediator.Send(new DeletePlayout(id), cancellationToken); return result.ToDeletedResult(); } @@ -722,7 +779,7 @@ public class PlayoutController(IMediator mediator) : ControllerBase vm.EndDay, vm.EndYear); - private static PlayoutListItemResponseModel ToListItemResponse(PlayoutNameViewModel vm) => + private static PlayoutListItemResponseModel ToListItemResponse(PlayoutNameViewModel vm, bool isLocked) => new( vm.PlayoutId, vm.ChannelNumber, @@ -731,7 +788,8 @@ public class PlayoutController(IMediator mediator) : ControllerBase vm.ScheduleName, vm.DbDailyRebuildTime, ToBuildStatus(vm.BuildStatus), - vm.PlayoutMode); + vm.PlayoutMode, + isLocked); private static PlayoutBuildStatusResponseModel ToBuildStatus(PlayoutBuildStatus buildStatus) => buildStatus is null diff --git a/ErsatzTV/Extensions/ApiResults.cs b/ErsatzTV/Extensions/ApiResults.cs index 8ec9a331f..ced802a41 100644 --- a/ErsatzTV/Extensions/ApiResults.cs +++ b/ErsatzTV/Extensions/ApiResults.cs @@ -50,6 +50,13 @@ public static class ApiResults public static IActionResult NotFoundProblem(string detail = "Resource not found") => new NotFoundObjectResult(CreateProblemDetails(404, "Resource not found", detail)); + /// + /// 409 directly — for a mutation that races a background operation + /// holding a lock (e.g. a playout build in flight). Mirrors . + /// + public static IActionResult ConflictProblem(string title, string detail) => + new ConflictObjectResult(CreateProblemDetails(409, title, detail)); + private static ProblemDetails CreateProblemDetails(int status, string title, string detail) => new() { diff --git a/ErsatzTV/wwwroot/openapi/v1.json b/ErsatzTV/wwwroot/openapi/v1.json index 847e90c5a..48d6b54aa 100644 --- a/ErsatzTV/wwwroot/openapi/v1.json +++ b/ErsatzTV/wwwroot/openapi/v1.json @@ -1764,6 +1764,26 @@ } } } + }, + "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" + } + } + } } } } @@ -7359,6 +7379,26 @@ } } }, + "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": { @@ -7421,6 +7461,26 @@ } } }, + "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": { @@ -7616,6 +7676,26 @@ } } }, + "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": { @@ -7822,6 +7902,26 @@ } } }, + "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": { @@ -8028,6 +8128,26 @@ } } }, + "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": { @@ -8353,6 +8473,26 @@ } } }, + "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": { @@ -8419,6 +8559,26 @@ } } }, + "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": { @@ -19667,7 +19827,8 @@ "scheduleName", "dailyRebuildTime", "buildStatus", - "playoutMode" + "playoutMode", + "isLocked" ], "type": "object", "properties": { @@ -19706,6 +19867,9 @@ }, "playoutMode": { "$ref": "#/components/schemas/ChannelPlayoutMode" + }, + "isLocked": { + "type": "boolean" } } }, diff --git a/docs/api-conventions.md b/docs/api-conventions.md index 2a89fe5b0..f83857f15 100644 --- a/docs/api-conventions.md +++ b/docs/api-conventions.md @@ -67,6 +67,27 @@ hand-rolling `IActionResult` status codes: | `ToDeletedResult()` | `Either` | `Left` → `ToErrorResult()`; `Right` → 204 | | `ToGetResult()` | `Option` | `Some` → 200 + body; `None` → 404 | | `ApiResults.NotFoundProblem(detail?)` | — | 404 `ProblemDetails` directly (e.g. when a controller has to pre-check existence itself, see `TemplateController.DeleteGroup`) | +| `ApiResults.ConflictProblem(title, detail)` | — | 409 `ProblemDetails` directly — for a mutation that races a background operation holding a lock (see §3a) | + +### 3a. 409 when a mutation races a background lock + +When an endpoint mutates an entity that a background operation may be actively rebuilding under an +`IEntityLocker` lock, guard the mutation and return **409 Conflict** (`ApiResults.ConflictProblem`) +while the lock is held, rather than letting the write race the build. This mirrors the Blazor UI, +which disables the same actions while the lock event is live. + +Established by issue #215 (`PlayoutController` + `ChannelController.ResetPlayout`): inject +`IEntityLocker`, and at the top of every id-keyed mutation (`PUT`/`POST`/`DELETE`) check +`IsPlayoutLocked(id)` → `ConflictProblem("Playout build in progress", ...)`; add +`[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. +- **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. `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 bb617695a..da8405fb3 100644 --- a/docs/blazor-route-parity.md +++ b/docs/blazor-route-parity.md @@ -59,7 +59,7 @@ redirect). > | Filler presets / Trakt / FFmpeg profiles | PARITY-OK | — | > | Watermarks | PARITY-OK (copy via `/add?from=` prefill, 2026-07-09) | — | > | Playout creation + alternate-schedules | PARITY-OK | — | -> | Playout kind-editors + list actions | PARITY-OK (delete/reset/erase/scheduling-context wired + templates preview calendar, 2026-07-09) | — | +> | Playout kind-editors + list actions | PARITY-OK (delete/reset/erase/scheduling-context wired + templates preview calendar, 2026-07-09; EntityLocker build-lock gating enforced server-side via 409 + mirrored in SPA `IsLocked`, 2026-07-10 #215) | — | > | Multi/rerun collections, playlists, trash | PARITY-OK (trash select-all/clear added; 100/kind cap → decisions.md) | — | > | Collections | PARITY-OK (custom-order endpoint + reorder UI, all-10-kind add picker, 2026-07-09) | — | > | Channel editor | GAPS (external logo URL, bare create, pickers) | #212 | @@ -192,6 +192,12 @@ query-wide Add All via `GET /api/search/all-items`, Save As Smart Collection) an pages + child grids, per-show Quick/Deep scan gated to Plex/Jellyfin/Emby, per-episode Media Info + Troubleshoot entries; `POST /api/playlists/{id}/items` added). Known accepted deviations (select-mode toggle, per-card target superset) recorded in `docs/decisions.md`. +CLOSED 2026-07-10: **#215** (adversarial-reviewer#18 removal gate) — Blazor's `EntityLocker` +build-lock gating of per-playout Reset/Erase/Delete/Edit is now enforced server-side: every +id-keyed `PlayoutController` mutation + `ChannelController.ResetPlayout` returns **409** while +`IsPlayoutLocked(id)`, and the SPA mirrors the lock via an `IsLocked` flag on the playout list +DTO (disables the buttons + shows a "Building…" cue). The safety invariant no longer depends on +Blazor, so its removal can't silently drop it. See `docs/api-conventions.md` §3a + `decisions.md`. ## Section 4 — Blazor home / escape hatch diff --git a/docs/decisions.md b/docs/decisions.md index 7dc4f6dec..d1180e032 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -225,3 +225,32 @@ TelevisionShow/TelevisionSeason/Artist per-media-item CollectionTypes and 422s t "Add All" (query-wide) mirrors Blazor's two-step: materialize ids via `GET /api/search/all-items`, then reuse the id-list add endpoints — no query-based add command exists server-side. Issues #208/#209. + +## 2026-07-10 — Playout API mutations return 409 while the build lock is held (#215) + +Blazor disabled per-playout Reset/Erase/Delete/Edit while a `BuildPlayout` was in flight +(`EntityLocker.IsPlayoutLocked`, `Playouts.razor` + per-kind editors); the REST API had no +equivalent, so a client could race an in-flight build with a destructive `ExecuteDelete` and leave +a half-built playout. Adversarial-reviewer#18 promoted this to a #91-phase-(b) removal gate: after +Blazor is deleted the invariant would vanish entirely. + +Decision: enforce the invariant **server-side** on the API rather than re-implementing a live push +channel. `PlayoutController` and `ChannelController` inject `IEntityLocker`; every id-keyed mutation +— `PUT /api/playouts/{id}`, `.../deco`, `.../alternate-schedules`, `.../templates`, +`POST .../erase-items`, `.../erase-items-and-history`, `DELETE /api/playouts/{id}`, and +`POST /api/channels/{channelNumber}/playout/reset` — checks `IsPlayoutLocked(id)` first and returns +**409 Conflict** (`ApiResults.ConflictProblem`, new shared helper mirroring `NotFoundProblem`) while +the build lock is held. The PUTs are gated too (not just the destructive ops): the invariant is "no +mutation during a build", matching Blazor's edit-disable. + +- **`reset-all` is deliberately NOT gated** — it stays 202. `ResetAllPlayoutsHandler` already + *silently skips* locked playouts, which matches Blazor and the handler semantics; a fire-and-forget + bulk enqueue always accepts. +- **SPA mirrors the lock via data, not a push channel** — `PlayoutListItemResponseModel` gains an + `IsLocked` bool (set from `IsPlayoutLocked` in the controller's list projection). The playouts + screen disables Reset/Erase/Erase-and-history/Delete for a locked row and shows a "Building…" + Badge; on a 409 from any mutation it surfaces the error and calls `query.refresh()` so the row + picks up the flag. No new polling was added (the existing 30s channel-state poll is unchanged). + +Precedent for the 409 shape: `TraktController` (left as-is with its own private `ConflictProblem()` +to keep the diff small). Convention recorded in `api-conventions.md` §3a. diff --git a/web/src/App.test.tsx b/web/src/App.test.tsx index 9d6bb692e..fd6beb59d 100644 --- a/web/src/App.test.tsx +++ b/web/src/App.test.tsx @@ -2005,6 +2005,25 @@ describe('ChicoryTV SPA scaffold', () => { expect(screen.queryByRole('button', { name: 'Erase items' })).not.toBeInTheDocument(); }); + it('disables mutation buttons and shows a Building cue for a locked (building) playout', async () => { + mockDashboardApi({ + playoutItems: [playoutItem()], + playoutDetails: playout({ id: 20, scheduleKind: 'Block' }), + playouts: { page: [listPlayout({ id: 20, isLocked: true, scheduleKind: 'Block' })], totalCount: 1 } + }); + + render(); + + fireEvent.click(screen.getByRole('link', { name: /Playouts/ })); + expect(await screen.findByRole('heading', { name: 'Retro Cartoons' })).toBeInTheDocument(); + + expect(screen.getByText('Building…')).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Reset' })).toBeDisabled(); + expect(screen.getByRole('button', { name: 'Erase items' })).toBeDisabled(); + expect(screen.getByRole('button', { name: 'Erase items and history' })).toBeDisabled(); + expect(screen.getByRole('button', { name: 'Delete' })).toBeDisabled(); + }); + it('disables Alternate schedules for an on-demand Classic playout', async () => { mockDashboardApi({ playoutItems: [playoutItem()], @@ -3531,6 +3550,7 @@ function listPlayout(overrides: Record = {}): Record { setMutationError(messageFromError(error)); + // A 409 means a build lock is now held for this playout; refresh so the row + // picks up its IsLocked state and the mutation buttons disable themselves. + if (error instanceof ApiError && error.status === 409) { + query.refresh(); + } }) .finally(() => { setMutatingState(false); @@ -3194,6 +3200,9 @@ function PlayoutsScreen() { } const selectedState = channelStates.find((state) => state.channelNumber === selectedSummary.channelNumber); + // The playout's build lock is held (a build is in flight). Server rejects destructive + // mutations with 409 while locked; mirror that by disabling the buttons here. + const selectedLocked = selectedSummary.isLocked; const nowPlaying = selectedState?.nowPlaying ?? null; const nowItem = itemMatchingNow(items, nowPlaying?.title) ?? items[0] ?? null; const nextItem = nextPlayoutItem(items, nowItem); @@ -3258,7 +3267,7 @@ function PlayoutsScreen() {
- {selectedSummary.channelNumber} {selectedState?.onAir && On air} + {selectedSummary.channelNumber} {selectedState?.onAir && On air} {selectedLocked && Building…}

{selectedSummary.channelName}

@@ -3346,20 +3355,22 @@ function PlayoutsScreen() { )}
{selectedSummary.scheduleKind === 'Block' && ( )}