diff --git a/ErsatzTV.Application/Scheduling/Commands/ReplaceDecoTemplateItems.cs b/ErsatzTV.Application/Scheduling/Commands/ReplaceDecoTemplateItems.cs index 9669e5223..85c178b2d 100644 --- a/ErsatzTV.Application/Scheduling/Commands/ReplaceDecoTemplateItems.cs +++ b/ErsatzTV.Application/Scheduling/Commands/ReplaceDecoTemplateItems.cs @@ -6,5 +6,6 @@ public record ReplaceDecoTemplateItems( int DecoTemplateId, int DecoTemplateGroupId, string Name, - List Items) + List Items, + Option ExpectedVersion = default) : IRequest>>; diff --git a/ErsatzTV.Application/Scheduling/Commands/ReplaceDecoTemplateItemsHandler.cs b/ErsatzTV.Application/Scheduling/Commands/ReplaceDecoTemplateItemsHandler.cs index 52fccc29d..d765ac468 100644 --- a/ErsatzTV.Application/Scheduling/Commands/ReplaceDecoTemplateItemsHandler.cs +++ b/ErsatzTV.Application/Scheduling/Commands/ReplaceDecoTemplateItemsHandler.cs @@ -20,10 +20,19 @@ public class ReplaceDecoTemplateItemsHandler( { await using TvContext dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); Validation validation = await Validate(dbContext, request, cancellationToken); - return await validation.Apply(ps => Persist(dbContext, request, ps, cancellationToken)); + + // Introduce the optimistic-concurrency check as a standalone Either AFTER the validation + // pipeline (never via Apply) so a PreconditionFailedError survives to a 412 and is not + // flattened to a generic 422 by Join() (issue #253 / api-conventions §7a). + Either validated = LanguageExtensions.ToEither(validation) + .Bind(decoTemplate => decoTemplate.CheckVersion(request.ExpectedVersion)); + + return await validated.Match( + Right: decoTemplate => Persist(dbContext, request, decoTemplate, cancellationToken), + Left: error => Task.FromResult>>(error)); } - private async Task> Persist( + private async Task>> Persist( TvContext dbContext, ReplaceDecoTemplateItems request, DecoTemplate decoTemplate, @@ -36,33 +45,49 @@ public class ReplaceDecoTemplateItemsHandler( decoTemplate.Items = request.Items.Map(i => BuildItem(decoTemplate, i)).ToList(); - await dbContext.SaveChangesAsync(cancellationToken); + // Unconditional bump: EF emits the root UPDATE only when a scalar actually differs, so a + // same-value/no-op save would otherwise write no root row and neither fire the concurrency + // token nor rotate other clients' ETags. Bumping guarantees both on every save (issue #253). + decoTemplate.Version++; - // Deco/break/default-filler content is only (re)applied during a Reset build (a Continue keeps the - // frozen DecoDefault filler items), and BlockKey change-detection has no deco dimension, so nothing - // self-heals a deco-template edit — the editor returned 200 but built filler stayed stale until a - // manual Reset (#251). Enqueue a Reset for every playout that references this deco template. This - // whole post-commit invalidation runs with CancellationToken.None (audit #22 policy): once the edit - // is committed, a late request cancellation must not be able to abort the affected-playout query OR - // the enqueue and leave content stale. - List playoutIds = await dbContext.PlayoutTemplates - .Where(pt => pt.DecoTemplateId == decoTemplate.Id) - .Select(pt => pt.PlayoutId) - .Distinct() - .ToListAsync(CancellationToken.None); + // Save through the guard so an EF concurrency failure (a racing writer won between our load + // and save) maps to 412 rather than surfacing as a 500. + Either saved = await dbContext.SaveChangesWithConcurrencyGuard(cancellationToken); + return await saved.Match( + Right: async _ => + { + // Deco/break/default-filler content is only (re)applied during a Reset build (a Continue keeps + // the frozen DecoDefault filler items), and BlockKey change-detection has no deco dimension, so + // nothing self-heals a deco-template edit — the editor returned 200 but built filler stayed + // stale until a manual Reset (#251). Enqueue a Reset for every playout that references this + // deco template. This whole post-commit invalidation runs with CancellationToken.None (audit + // #22 policy): once the edit is committed, a late request cancellation must not be able to + // abort the affected-playout query OR the enqueue and leave content stale. Only runs after a + // successful save (issue #253) — a 412/422 must not enqueue a Reset for content that was never + // persisted. + List playoutIds = await dbContext.PlayoutTemplates + .Where(pt => pt.DecoTemplateId == decoTemplate.Id) + .Select(pt => pt.PlayoutId) + .Distinct() + .ToListAsync(CancellationToken.None); - foreach (int playoutId in playoutIds) - { - await channel.WriteAsync(new BuildPlayout(playoutId, PlayoutBuildMode.Reset), CancellationToken.None); - } + foreach (int playoutId in playoutIds) + { + await channel.WriteAsync( + new BuildPlayout(playoutId, PlayoutBuildMode.Reset), + CancellationToken.None); + } - await dbContext.Entry(decoTemplate) - .Collection(t => t.Items) - .Query() - .Include(i => i.Deco) - .LoadAsync(cancellationToken); + await dbContext.Entry(decoTemplate) + .Collection(t => t.Items) + .Query() + .Include(i => i.Deco) + .LoadAsync(cancellationToken); - return decoTemplate.Items.Map(Mapper.ProjectToViewModel).ToList(); + return Right>( + decoTemplate.Items.Map(Mapper.ProjectToViewModel).ToList()); + }, + Left: error => Task.FromResult(Left>(error))); } private static DecoTemplateItem BuildItem(DecoTemplate decoTemplate, ReplaceDecoTemplateItem item) => diff --git a/ErsatzTV.Application/Scheduling/Commands/ReplaceTemplateItems.cs b/ErsatzTV.Application/Scheduling/Commands/ReplaceTemplateItems.cs index e00cfef5f..f8ff2c1a8 100644 --- a/ErsatzTV.Application/Scheduling/Commands/ReplaceTemplateItems.cs +++ b/ErsatzTV.Application/Scheduling/Commands/ReplaceTemplateItems.cs @@ -2,5 +2,10 @@ using ErsatzTV.Core; namespace ErsatzTV.Application.Scheduling; -public record ReplaceTemplateItems(int TemplateGroupId, int TemplateId, string Name, List Items) +public record ReplaceTemplateItems( + int TemplateGroupId, + int TemplateId, + string Name, + List Items, + Option ExpectedVersion = default) : IRequest>>; diff --git a/ErsatzTV.Application/Scheduling/Commands/ReplaceTemplateItemsHandler.cs b/ErsatzTV.Application/Scheduling/Commands/ReplaceTemplateItemsHandler.cs index 055f3c0e4..bc67427e2 100644 --- a/ErsatzTV.Application/Scheduling/Commands/ReplaceTemplateItemsHandler.cs +++ b/ErsatzTV.Application/Scheduling/Commands/ReplaceTemplateItemsHandler.cs @@ -15,10 +15,19 @@ public class ReplaceTemplateItemsHandler(IDbContextFactory dbContextF { await using TvContext dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); Validation validation = await Validate(dbContext, request, cancellationToken); - return await validation.Apply(ps => Persist(dbContext, request, ps, cancellationToken)); + + // Introduce the optimistic-concurrency check as a standalone Either AFTER the validation + // pipeline (never via Apply) so a PreconditionFailedError survives to a 412 and is not + // flattened to a generic 422 by Join() (issue #253 / api-conventions §7a). + Either validated = LanguageExtensions.ToEither(validation) + .Bind(template => template.CheckVersion(request.ExpectedVersion)); + + return await validated.Match( + Right: template => Persist(dbContext, request, template, cancellationToken), + Left: error => Task.FromResult>>(error)); } - private static async Task> Persist( + private static async Task>> Persist( TvContext dbContext, ReplaceTemplateItems request, Template template, @@ -30,7 +39,10 @@ public class ReplaceTemplateItemsHandler(IDbContextFactory dbContextF dbContext.RemoveRange(template.Items); template.Items = request.Items.Map(i => BuildItem(template, i)).ToList(); - await dbContext.SaveChangesAsync(cancellationToken); + // Unconditional bump: EF emits the root UPDATE only when a scalar actually differs, so a + // same-value/no-op save would otherwise write no root row and neither fire the concurrency + // token nor rotate other clients' ETags. Bumping guarantees both on every save (issue #253). + template.Version++; // TODO: refresh any playouts that use this schedule // foreach (Playout playout in programSchedule.Playouts) @@ -38,13 +50,22 @@ public class ReplaceTemplateItemsHandler(IDbContextFactory dbContextF // await _channel.WriteAsync(new BuildPlayout(playout.Id, PlayoutBuildMode.Refresh)); // } - await dbContext.Entry(template) - .Collection(t => t.Items) - .Query() - .Include(i => i.Block) - .LoadAsync(cancellationToken); + // Save through the guard so an EF concurrency failure (a racing writer won between our load + // and save) maps to 412 rather than surfacing as a 500. + Either saved = await dbContext.SaveChangesWithConcurrencyGuard(cancellationToken); + return await saved.Match( + Right: async _ => + { + await dbContext.Entry(template) + .Collection(t => t.Items) + .Query() + .Include(i => i.Block) + .LoadAsync(cancellationToken); - return template.Items.Map(Mapper.ProjectToViewModel).ToList(); + return Right>( + template.Items.Map(Mapper.ProjectToViewModel).ToList()); + }, + Left: error => Task.FromResult(Left>(error))); } private static TemplateItem BuildItem(Template template, ReplaceTemplateItem item) => diff --git a/ErsatzTV.Application/Scheduling/DecoTemplateViewModel.cs b/ErsatzTV.Application/Scheduling/DecoTemplateViewModel.cs index c9543d63c..d1517d1c2 100644 --- a/ErsatzTV.Application/Scheduling/DecoTemplateViewModel.cs +++ b/ErsatzTV.Application/Scheduling/DecoTemplateViewModel.cs @@ -1,3 +1,3 @@ namespace ErsatzTV.Application.Scheduling; -public record DecoTemplateViewModel(int Id, int DecoTemplateGroupId, string GroupName, string Name); +public record DecoTemplateViewModel(int Id, int DecoTemplateGroupId, string GroupName, string Name, int Version); diff --git a/ErsatzTV.Application/Scheduling/Mapper.cs b/ErsatzTV.Application/Scheduling/Mapper.cs index d82d9325f..91e6356b5 100644 --- a/ErsatzTV.Application/Scheduling/Mapper.cs +++ b/ErsatzTV.Application/Scheduling/Mapper.cs @@ -65,7 +65,7 @@ internal static class Mapper new(templateGroup.Id, templateGroup.Name, templateGroup.Templates.Count); internal static TemplateViewModel ProjectToViewModel(Template template) => - new(template.Id, template.TemplateGroupId, template.TemplateGroup.Name, template.Name); + new(template.Id, template.TemplateGroupId, template.TemplateGroup.Name, template.Name, template.Version); internal static TemplateItemViewModel ProjectToViewModel(TemplateItem templateItem) { @@ -168,7 +168,8 @@ internal static class Mapper decoTemplate.Id, decoTemplate.DecoTemplateGroupId, decoTemplate.DecoTemplateGroup.Name, - decoTemplate.Name); + decoTemplate.Name, + decoTemplate.Version); } internal static DecoTemplateItemViewModel ProjectToViewModel(DecoTemplateItem decoTemplateItem) diff --git a/ErsatzTV.Application/Scheduling/TemplateViewModel.cs b/ErsatzTV.Application/Scheduling/TemplateViewModel.cs index cef2789bf..1e72952b0 100644 --- a/ErsatzTV.Application/Scheduling/TemplateViewModel.cs +++ b/ErsatzTV.Application/Scheduling/TemplateViewModel.cs @@ -1,3 +1,3 @@ namespace ErsatzTV.Application.Scheduling; -public record TemplateViewModel(int Id, int TemplateGroupId, string GroupName, string Name); +public record TemplateViewModel(int Id, int TemplateGroupId, string GroupName, string Name, int Version); diff --git a/ErsatzTV.Tests/Application/Scheduling/ReplaceDecoTemplateItemsHandlerConcurrencyTests.cs b/ErsatzTV.Tests/Application/Scheduling/ReplaceDecoTemplateItemsHandlerConcurrencyTests.cs new file mode 100644 index 000000000..3d265d0c8 --- /dev/null +++ b/ErsatzTV.Tests/Application/Scheduling/ReplaceDecoTemplateItemsHandlerConcurrencyTests.cs @@ -0,0 +1,164 @@ +using System.Threading.Channels; +using ErsatzTV.Application; +using ErsatzTV.Application.Playouts; +using ErsatzTV.Application.Scheduling; +using ErsatzTV.Core; +using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Domain.Scheduling; +using ErsatzTV.Core.Errors; +using ErsatzTV.Infrastructure.Data; +using ErsatzTV.Tests.Support; +using LanguageExt; +using Microsoft.EntityFrameworkCore; +using NUnit.Framework; +using Shouldly; +using static LanguageExt.Prelude; + +namespace ErsatzTV.Tests.Application.Scheduling; + +/// +/// Contract tests for the #253 optimistic-concurrency mechanic on the DecoTemplate aggregate, +/// mirroring (the Block reference +/// implementation): the handler pre-check (stale If-Match → 412), the force-write path (no +/// If-Match), the unconditional Version bump on every save, and the EF concurrency-token backstop +/// that catches a writer that lost the load→save race. The backstop test is non-vacuous by +/// construction — remove the IsConcurrencyToken() config on DecoTemplate and the losing save +/// silently succeeds instead of mapping to a . +/// +[TestFixture] +public class ReplaceDecoTemplateItemsHandlerConcurrencyTests +{ + private InMemoryTvContext _db = null!; + private Channel _channel = null!; + + [SetUp] + public async Task SetUp() + { + _db = await InMemoryTvContext.CreateAsync(); + _channel = System.Threading.Channels.Channel.CreateUnbounded(); + } + + [TearDown] + public async Task TearDown() => await _db.DisposeAsync(); + + private async Task SeedDecoTemplateAsync(int version) + { + await using TvContext ctx = _db.CreateContext(); + ctx.Decos.Add(new Deco { Id = 10, DecoGroupId = 1, Name = "D" }); + ctx.DecoTemplates.Add( + new DecoTemplate + { + Id = 1, + DecoTemplateGroupId = 1, + Name = "Weekday", + Version = version, + Items = new List() + }); + await ctx.SaveChangesAsync(); + } + + private ReplaceDecoTemplateItemsHandler CreateHandler() => new(_db.Factory, _channel.Writer); + + private static ReplaceDecoTemplateItems Command(Option expectedVersion) => + new( + DecoTemplateId: 1, + DecoTemplateGroupId: 1, + Name: "Weekday", + Items: [new ReplaceDecoTemplateItem(DecoId: 10, StartTime: TimeSpan.Zero, EndTime: TimeSpan.FromHours(1))], + ExpectedVersion: expectedVersion); + + private async Task ReadVersionAsync() + { + await using TvContext ctx = _db.CreateContext(); + return await ctx.DecoTemplates.Where(t => t.Id == 1).Select(t => t.Version).SingleAsync(); + } + + private static BaseError? LeftOrNull(Either result) => + result.Match(Right: _ => null, Left: e => e); + + [Test] + public async Task Stale_If_Match_Should_Fail_Precondition_And_Not_Mutate() + { + await SeedDecoTemplateAsync(version: 2); + ReplaceDecoTemplateItemsHandler handler = CreateHandler(); + + Either> result = + await handler.Handle(Command(Some(1)), CancellationToken.None); + + LeftOrNull(result).ShouldBeOfType(); + + // The pre-check runs before any mutation: version unchanged, no items written. + (await ReadVersionAsync()).ShouldBe(2); + await using TvContext ctx = _db.CreateContext(); + (await ctx.DecoTemplateItems.CountAsync(i => i.DecoTemplateId == 1)).ShouldBe(0); + } + + [Test] + public async Task Matching_If_Match_Should_Succeed_And_Bump_Version() + { + await SeedDecoTemplateAsync(version: 2); + ReplaceDecoTemplateItemsHandler handler = CreateHandler(); + + Either> result = + await handler.Handle(Command(Some(2)), CancellationToken.None); + + result.IsRight.ShouldBeTrue(); + (await ReadVersionAsync()).ShouldBe(3); + } + + [Test] + public async Task Absent_If_Match_Should_Force_Write_And_Bump_Version() + { + await SeedDecoTemplateAsync(version: 2); + ReplaceDecoTemplateItemsHandler handler = CreateHandler(); + + // None expected version = Phase-1 force-write regardless of the stored version. + Either> result = + await handler.Handle(Command(None), CancellationToken.None); + + result.IsRight.ShouldBeTrue(); + (await ReadVersionAsync()).ShouldBe(3); + } + + [Test] + public async Task Save_Should_Bump_Version_Even_When_Content_Is_Unchanged() + { + await SeedDecoTemplateAsync(version: 5); + ReplaceDecoTemplateItemsHandler handler = CreateHandler(); + + // Same content twice: the unconditional bump (M1) must still rotate the version each time, + // otherwise a no-op PUT-back would not fire the token or rotate other clients' ETags. + (await handler.Handle(Command(Some(5)), CancellationToken.None)).IsRight.ShouldBeTrue(); + (await ReadVersionAsync()).ShouldBe(6); + (await handler.Handle(Command(Some(6)), CancellationToken.None)).IsRight.ShouldBeTrue(); + (await ReadVersionAsync()).ShouldBe(7); + } + + [Test] + public async Task Racing_Save_Should_Map_Concurrency_Failure_To_412() + { + await SeedDecoTemplateAsync(version: 1); + + // Two writers load the same version, then both bump-and-save. The IsConcurrencyToken guard on + // DecoTemplate makes the second UPDATE key on the original version; it matches zero rows and + // throws DbUpdateConcurrencyException, which the shared save helper maps to a + // PreconditionFailedError. + await using TvContext ctxWinner = _db.CreateContext(); + await using TvContext ctxLoser = _db.CreateContext(); + + DecoTemplate winner = await ctxWinner.DecoTemplates.SingleAsync(t => t.Id == 1); + DecoTemplate loser = await ctxLoser.DecoTemplates.SingleAsync(t => t.Id == 1); + + winner.Version++; + Either winnerResult = await ctxWinner.SaveChangesWithConcurrencyGuard(CancellationToken.None); + winnerResult.IsRight.ShouldBeTrue(); + + loser.Version++; + Either loserResult = await ctxLoser.SaveChangesWithConcurrencyGuard(CancellationToken.None); + + LeftOrNull(loserResult).ShouldBeOfType(); + + // Non-vacuous: the winner's write stuck at exactly its +1; the loser did not overwrite it. + (await ReadVersionAsync()).ShouldBe(2); + } +} diff --git a/ErsatzTV.Tests/Application/Scheduling/ReplaceTemplateItemsHandlerConcurrencyTests.cs b/ErsatzTV.Tests/Application/Scheduling/ReplaceTemplateItemsHandlerConcurrencyTests.cs new file mode 100644 index 000000000..4e71c0a91 --- /dev/null +++ b/ErsatzTV.Tests/Application/Scheduling/ReplaceTemplateItemsHandlerConcurrencyTests.cs @@ -0,0 +1,163 @@ +using ErsatzTV.Application; +using ErsatzTV.Application.Scheduling; +using ErsatzTV.Core; +using ErsatzTV.Core.Domain.Scheduling; +using ErsatzTV.Core.Errors; +using ErsatzTV.Infrastructure.Data; +using ErsatzTV.Tests.Support; +using LanguageExt; +using Microsoft.EntityFrameworkCore; +using NUnit.Framework; +using Shouldly; +using static LanguageExt.Prelude; + +namespace ErsatzTV.Tests.Application.Scheduling; + +/// +/// Contract tests for the #253 optimistic-concurrency mechanic on the Template aggregate, +/// mirroring (the Block reference +/// implementation): the handler pre-check (stale If-Match → 412), the force-write path (no +/// If-Match), the unconditional Version bump on every save, and the EF concurrency-token backstop +/// that catches a writer that lost the load→save race. The backstop test is non-vacuous by +/// construction — remove the IsConcurrencyToken() config on Template and the losing save +/// silently succeeds instead of mapping to a . +/// +[TestFixture] +public class ReplaceTemplateItemsHandlerConcurrencyTests +{ + private InMemoryTvContext _db = null!; + + [SetUp] + public async Task SetUp() => _db = await InMemoryTvContext.CreateAsync(); + + [TearDown] + public async Task TearDown() => await _db.DisposeAsync(); + + private async Task SeedTemplateAsync(int version) + { + await using TvContext ctx = _db.CreateContext(); + ctx.Blocks.Add( + new Block + { + Id = 10, + BlockGroupId = 1, + Name = "Morning", + Minutes = 30, + StopScheduling = BlockStopScheduling.AfterDurationEnd, + Items = new List() + }); + ctx.Templates.Add( + new Template + { + Id = 1, + TemplateGroupId = 1, + Name = "Weekday", + Version = version, + Items = new List() + }); + await ctx.SaveChangesAsync(); + } + + private static ReplaceTemplateItems Command(Option expectedVersion) => + new( + 1, + 1, + "Weekday", + new List { new(10, TimeSpan.Zero) }, + expectedVersion); + + private async Task ReadVersionAsync() + { + await using TvContext ctx = _db.CreateContext(); + return await ctx.Templates.Where(t => t.Id == 1).Select(t => t.Version).SingleAsync(); + } + + private static BaseError? LeftOrNull(Either result) => + result.Match(Right: _ => null, Left: e => e); + + [Test] + public async Task Stale_If_Match_Should_Fail_Precondition_And_Not_Mutate() + { + await SeedTemplateAsync(version: 2); + var handler = new ReplaceTemplateItemsHandler(_db.Factory); + + Either> result = + await handler.Handle(Command(Some(1)), CancellationToken.None); + + LeftOrNull(result).ShouldBeOfType(); + + // The pre-check runs before any mutation: version unchanged, no items written. + (await ReadVersionAsync()).ShouldBe(2); + await using TvContext ctx = _db.CreateContext(); + (await ctx.TemplateItems.CountAsync(i => i.TemplateId == 1)).ShouldBe(0); + } + + [Test] + public async Task Matching_If_Match_Should_Succeed_And_Bump_Version() + { + await SeedTemplateAsync(version: 2); + var handler = new ReplaceTemplateItemsHandler(_db.Factory); + + Either> result = + await handler.Handle(Command(Some(2)), CancellationToken.None); + + result.IsRight.ShouldBeTrue(); + (await ReadVersionAsync()).ShouldBe(3); + } + + [Test] + public async Task Absent_If_Match_Should_Force_Write_And_Bump_Version() + { + await SeedTemplateAsync(version: 2); + var handler = new ReplaceTemplateItemsHandler(_db.Factory); + + // None expected version = Phase-1 force-write regardless of the stored version. + Either> result = + await handler.Handle(Command(None), CancellationToken.None); + + result.IsRight.ShouldBeTrue(); + (await ReadVersionAsync()).ShouldBe(3); + } + + [Test] + public async Task Save_Should_Bump_Version_Even_When_Content_Is_Unchanged() + { + await SeedTemplateAsync(version: 5); + var handler = new ReplaceTemplateItemsHandler(_db.Factory); + + // Same content twice: the unconditional bump (M1) must still rotate the version each time, + // otherwise a no-op PUT-back would not fire the token or rotate other clients' ETags. + (await handler.Handle(Command(Some(5)), CancellationToken.None)).IsRight.ShouldBeTrue(); + (await ReadVersionAsync()).ShouldBe(6); + (await handler.Handle(Command(Some(6)), CancellationToken.None)).IsRight.ShouldBeTrue(); + (await ReadVersionAsync()).ShouldBe(7); + } + + [Test] + public async Task Racing_Save_Should_Map_Concurrency_Failure_To_412() + { + await SeedTemplateAsync(version: 1); + + // Two writers load the same version, then both bump-and-save. The IsConcurrencyToken guard on + // Template makes the second UPDATE key on the original version; it matches zero rows and + // throws DbUpdateConcurrencyException, which the shared save helper maps to a + // PreconditionFailedError. + await using TvContext ctxWinner = _db.CreateContext(); + await using TvContext ctxLoser = _db.CreateContext(); + + Template winner = await ctxWinner.Templates.SingleAsync(t => t.Id == 1); + Template loser = await ctxLoser.Templates.SingleAsync(t => t.Id == 1); + + winner.Version++; + Either winnerResult = await ctxWinner.SaveChangesWithConcurrencyGuard(CancellationToken.None); + winnerResult.IsRight.ShouldBeTrue(); + + loser.Version++; + Either loserResult = await ctxLoser.SaveChangesWithConcurrencyGuard(CancellationToken.None); + + LeftOrNull(loserResult).ShouldBeOfType(); + + // Non-vacuous: the winner's write stuck at exactly its +1; the loser did not overwrite it. + (await ReadVersionAsync()).ShouldBe(2); + } +} diff --git a/ErsatzTV.Tests/Controllers/DecoTemplateControllerTests.cs b/ErsatzTV.Tests/Controllers/DecoTemplateControllerTests.cs index 6ebfe6754..044c83450 100644 --- a/ErsatzTV.Tests/Controllers/DecoTemplateControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/DecoTemplateControllerTests.cs @@ -4,8 +4,10 @@ using ErsatzTV.Controllers.Api; using ErsatzTV.Controllers.Api.Requests; using ErsatzTV.Core; using ErsatzTV.Core.Api.Scheduling; +using ErsatzTV.Core.Errors; using LanguageExt; using MediatR; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Routing; using NSubstitute; @@ -25,7 +27,12 @@ public class DecoTemplateControllerTests public void SetUp() { _mediator = Substitute.For(); - _controller = new DecoTemplateController(_mediator); + _controller = new DecoTemplateController(_mediator) + { + // Provide a real HttpContext so the ETag/If-Match concurrency headers (#253) can be + // read from Request and written to Response. + ControllerContext = new ControllerContext { HttpContext = new DefaultHttpContext() } + }; } [Test] @@ -259,12 +266,8 @@ public class DecoTemplateControllerTests _mediator.Send(Arg.Any(), Arg.Any()) .Returns(Option.Some(MakeDecoTemplate(4, 7, "Morning"))); _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Right>([])); - _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(new List - { - MakeItem(10, "Morning Bumper", TimeSpan.FromHours(6), TimeSpan.FromHours(7)) - }); + .Returns(Right>( + [MakeItem(10, "Morning Bumper", TimeSpan.FromHours(6), TimeSpan.FromHours(7))])); IActionResult result = await _controller.Replace( 4, @@ -318,11 +321,96 @@ public class DecoTemplateControllerTests CancellationToken.None); result.ShouldBeOfType(); - await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); } - private static DecoTemplateViewModel MakeDecoTemplate(int id, int groupId, string name) => - new(id, groupId, "Group", name); + [Test] + public async Task GetItems_Should_Set_ETag_From_DecoTemplate_Version() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(MakeDecoTemplate(4, 2, "Morning", version: 9))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(new List()); + + await _controller.GetItems(4, CancellationToken.None); + + _controller.Response.Headers.ETag.ToString().ShouldBe("\"9\""); + } + + [Test] + public async Task Replace_Should_Return_400_On_Malformed_If_Match() + { + _controller.Request.Headers.IfMatch = "not-an-etag"; + + IActionResult result = await _controller.Replace( + 4, + new ReplaceDecoTemplateRequest("Morning", []), + CancellationToken.None); + + result.ShouldBeOfType(); + await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + } + + [Test] + public async Task Replace_Should_Thread_If_Match_Version_Into_Command() + { + _controller.Request.Headers.IfMatch = "\"3\""; + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(MakeDecoTemplate(4, 7, "Morning", version: 4))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Right>([])); + + IActionResult result = await _controller.Replace( + 4, + new ReplaceDecoTemplateRequest("Morning", []), + CancellationToken.None); + + result.ShouldBeOfType(); + // On success the response carries the refreshed deco template's ETag. + _controller.Response.Headers.ETag.ToString().ShouldBe("\"4\""); + await _mediator.Received(1).Send( + Arg.Is(c => c.ExpectedVersion == Option.Some(3)), + Arg.Any()); + } + + [Test] + public async Task Replace_Without_If_Match_Should_Force_Write() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(MakeDecoTemplate(4, 7, "Morning"))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Right>([])); + + await _controller.Replace( + 4, + new ReplaceDecoTemplateRequest("Morning", []), + CancellationToken.None); + + await _mediator.Received(1).Send( + Arg.Is(c => c.ExpectedVersion == Option.None), + Arg.Any()); + } + + [Test] + public async Task Replace_Should_Return_412_On_Precondition_Failed() + { + _controller.Request.Headers.IfMatch = "\"2\""; + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(MakeDecoTemplate(4, 7, "Morning", version: 5))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Left>(new PreconditionFailedError("stale"))); + + IActionResult result = await _controller.Replace( + 4, + new ReplaceDecoTemplateRequest("Morning", []), + CancellationToken.None); + + var objectResult = result.ShouldBeOfType(); + objectResult.StatusCode.ShouldBe(StatusCodes.Status412PreconditionFailed); + } + + private static DecoTemplateViewModel MakeDecoTemplate(int id, int groupId, string name, int version = 1) => + new(id, groupId, "Group", name, version); private static DecoTemplateItemViewModel MakeItem(int decoId, string decoName, TimeSpan startTime, TimeSpan endTime) { diff --git a/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs b/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs index d0af315c4..a93e0c1a4 100644 --- a/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs @@ -1108,7 +1108,7 @@ public class PlayoutControllerTests _mediator.Send(Arg.Any(), Arg.Any()) .Returns([MakeTemplateViewModel(7), MakeTemplateViewModel(8)]); _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Option.Some(new DecoTemplateViewModel(5, 1, "G", "DT"))); + .Returns(Option.Some(new DecoTemplateViewModel(5, 1, "G", "DT", 0))); _mediator.Send(Arg.Any(), Arg.Any()) .Returns(Option.None); _mediator.Send(Arg.Any(), Arg.Any()) @@ -1250,13 +1250,13 @@ public class PlayoutControllerTests new(id, $"Schedule {id}", false, false, false, false, FixedStartTimeBehavior.Strict); private static TemplateViewModel MakeTemplateViewModel(int id) => - new(id, 1, "Group", $"Template {id}"); + new(id, 1, "Group", $"Template {id}", 0); private static PlayoutTemplateViewModel MakeTemplateVm(int id, int index, int templateId, int? decoTemplateId) => new( id, - new TemplateViewModel(templateId, 1, "Group", $"Template {templateId}"), - decoTemplateId is { } dtId ? new DecoTemplateViewModel(dtId, 1, "DGroup", $"Deco {dtId}") : null, + new TemplateViewModel(templateId, 1, "Group", $"Template {templateId}", 0), + decoTemplateId is { } dtId ? new DecoTemplateViewModel(dtId, 1, "DGroup", $"Deco {dtId}", 0) : null, index, [], [], diff --git a/ErsatzTV.Tests/Controllers/TemplateControllerTests.cs b/ErsatzTV.Tests/Controllers/TemplateControllerTests.cs index a56f47017..4a5f9c595 100644 --- a/ErsatzTV.Tests/Controllers/TemplateControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/TemplateControllerTests.cs @@ -4,8 +4,10 @@ using ErsatzTV.Controllers.Api; using ErsatzTV.Controllers.Api.Requests; using ErsatzTV.Core; using ErsatzTV.Core.Api.Scheduling; +using ErsatzTV.Core.Errors; using LanguageExt; using MediatR; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Routing; using NSubstitute; @@ -25,7 +27,12 @@ public class TemplateControllerTests public void SetUp() { _mediator = Substitute.For(); - _controller = new TemplateController(_mediator); + _controller = new TemplateController(_mediator) + { + // Provide a real HttpContext so the ETag/If-Match concurrency headers (#253) can be + // read from Request and written to Response. + ControllerContext = new ControllerContext { HttpContext = new DefaultHttpContext() } + }; } [Test] @@ -248,12 +255,8 @@ public class TemplateControllerTests _mediator.Send(Arg.Any(), Arg.Any()) .Returns(Option.Some(MakeTemplate(4, 7, "Morning"))); _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Right>([])); - _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(new List - { - MakeItem(10, "Cartoons", TimeSpan.FromHours(6), 60) - }); + .Returns(Right>( + [MakeItem(10, "Cartoons", TimeSpan.FromHours(6), 60)])); IActionResult result = await _controller.Replace( 4, @@ -305,7 +308,92 @@ public class TemplateControllerTests CancellationToken.None); result.ShouldBeOfType(); - await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + } + + [Test] + public async Task GetItems_Should_Set_ETag_From_Template_Version() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(MakeTemplate(4, 2, "Morning", version: 9))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(new List()); + + await _controller.GetItems(4, CancellationToken.None); + + _controller.Response.Headers.ETag.ToString().ShouldBe("\"9\""); + } + + [Test] + public async Task Replace_Should_Return_400_On_Malformed_If_Match() + { + _controller.Request.Headers.IfMatch = "not-an-etag"; + + IActionResult result = await _controller.Replace( + 4, + new ReplaceTemplateRequest("Morning", []), + CancellationToken.None); + + result.ShouldBeOfType(); + await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + } + + [Test] + public async Task Replace_Should_Thread_If_Match_Version_Into_Command() + { + _controller.Request.Headers.IfMatch = "\"3\""; + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(MakeTemplate(4, 7, "Morning", version: 4))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Right>([])); + + IActionResult result = await _controller.Replace( + 4, + new ReplaceTemplateRequest("Morning", []), + CancellationToken.None); + + result.ShouldBeOfType(); + // On success the response carries the refreshed template's ETag. + _controller.Response.Headers.ETag.ToString().ShouldBe("\"4\""); + await _mediator.Received(1).Send( + Arg.Is(c => c.ExpectedVersion == Option.Some(3)), + Arg.Any()); + } + + [Test] + public async Task Replace_Without_If_Match_Should_Force_Write() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(MakeTemplate(4, 7, "Morning"))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Right>([])); + + await _controller.Replace( + 4, + new ReplaceTemplateRequest("Morning", []), + CancellationToken.None); + + await _mediator.Received(1).Send( + Arg.Is(c => c.ExpectedVersion == Option.None), + Arg.Any()); + } + + [Test] + public async Task Replace_Should_Return_412_On_Precondition_Failed() + { + _controller.Request.Headers.IfMatch = "\"2\""; + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(MakeTemplate(4, 7, "Morning", version: 5))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Left>(new PreconditionFailedError("stale"))); + + IActionResult result = await _controller.Replace( + 4, + new ReplaceTemplateRequest("Morning", []), + CancellationToken.None); + + var objectResult = result.ShouldBeOfType(); + objectResult.StatusCode.ShouldBe(StatusCodes.Status412PreconditionFailed); } [Test] @@ -359,8 +447,8 @@ public class TemplateControllerTests result.ShouldBeOfType(); } - private static TemplateViewModel MakeTemplate(int id, int groupId, string name) => - new(id, groupId, "Group", name); + private static TemplateViewModel MakeTemplate(int id, int groupId, string name, int version = 1) => + new(id, groupId, "Group", name, version); private static TemplateItemViewModel MakeItem(int blockId, string blockName, TimeSpan startTime, int minutes) { diff --git a/ErsatzTV/Controllers/Api/DecoTemplateController.cs b/ErsatzTV/Controllers/Api/DecoTemplateController.cs index 82a316239..1c8dc80fb 100644 --- a/ErsatzTV/Controllers/Api/DecoTemplateController.cs +++ b/ErsatzTV/Controllers/Api/DecoTemplateController.cs @@ -143,6 +143,9 @@ public class DecoTemplateController(IMediator mediator) : ControllerBase [HttpGet("/api/deco-templates/{id:int}/items")] [Tags("DecoTemplates")] [EndpointSummary("Get deco template items")] + [EndpointDescription( + "Returns the deco template's items and a strong ETag of the deco template's version. Pass that ETag " + + "back as If-Match on the replace (PUT) to detect a concurrent edit (issue #253).")] [EndpointGroupName("general")] [ProducesResponseType(typeof(List), StatusCodes.Status200OK)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] @@ -154,6 +157,9 @@ public class DecoTemplateController(IMediator mediator) : ControllerBase return ApiResults.NotFoundProblem(); } + // The items GET returns children, not the root, so read the deco template's version for the ETag. + ConcurrencyHeaders.SetETag(Response, decoTemplate.Map(t => t.Version).IfNone(0)); + List items = await mediator.Send(new GetDecoTemplateItems(id), cancellationToken); return new OkObjectResult( items.OrderBy(i => i.StartTime.TimeOfDay).Map(ProjectToResponseModel).ToList()); @@ -164,16 +170,32 @@ public class DecoTemplateController(IMediator mediator) : ControllerBase [EndpointSummary("Replace a deco template and its items")] [EndpointDescription( "Replaces the deco template's name and its full item list. Each item assigns a deco to a start/end time " + - "of day; an end time of 00:00:00 means the item runs to the end of the day. Items must not overlap.")] + "of day; an end time of 00:00:00 means the item runs to the end of the day. Items must not overlap. " + + "Send the ETag from the items GET as If-Match to reject a stale overwrite with 412 (issue #253); " + + "a successful response carries the new ETag.")] [EndpointGroupName("general")] [ProducesResponseType(typeof(DecoTemplateWithItemsResponseModel), StatusCodes.Status200OK)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status400BadRequest)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status412PreconditionFailed)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task Replace( int id, [Required] [FromBody] ReplaceDecoTemplateRequest request, CancellationToken cancellationToken) { + IfMatchCondition ifMatch = ConcurrencyHeaders.ParseIfMatch(Request); + if (ifMatch.Kind is IfMatchKind.Malformed) + { + return new BadRequestObjectResult( + new ProblemDetails + { + Status = StatusCodes.Status400BadRequest, + Title = "Invalid If-Match header", + Detail = "If-Match must be a strong ETag of the resource version (e.g. \"3\") or \"*\"." + }); + } + Option maybeDecoTemplate = await mediator.Send(new GetDecoTemplateById(id), cancellationToken); if (maybeDecoTemplate.IsNone) @@ -184,18 +206,23 @@ public class DecoTemplateController(IMediator mediator) : ControllerBase int decoTemplateGroupId = maybeDecoTemplate.Map(t => t.DecoTemplateGroupId).IfNone(0); Either> result = - await mediator.Send(request.ToCommand(decoTemplateGroupId, id), cancellationToken); + await mediator.Send( + request.ToCommand(decoTemplateGroupId, id, ifMatch.ExpectedVersion), + cancellationToken); return await result.Match( Left: error => Task.FromResult(error.ToErrorResult()), - Right: async _ => + Right: async items => { Option refreshed = await mediator.Send(new GetDecoTemplateById(id), cancellationToken); - List items = - await mediator.Send(new GetDecoTemplateItems(id), cancellationToken); return refreshed.Match( - Some: vm => (IActionResult)new OkObjectResult(ProjectToWithItemsResponseModel(vm, items)), + Some: vm => + { + // Return the new ETag so a same-tab second save doesn't 412 against its own write. + ConcurrencyHeaders.SetETag(Response, vm.Version); + return (IActionResult)new OkObjectResult(ProjectToWithItemsResponseModel(vm, items)); + }, None: () => ApiResults.NotFoundProblem()); }); } diff --git a/ErsatzTV/Controllers/Api/Requests/ReplaceDecoTemplateRequest.cs b/ErsatzTV/Controllers/Api/Requests/ReplaceDecoTemplateRequest.cs index e649a6a1e..734655928 100644 --- a/ErsatzTV/Controllers/Api/Requests/ReplaceDecoTemplateRequest.cs +++ b/ErsatzTV/Controllers/Api/Requests/ReplaceDecoTemplateRequest.cs @@ -4,10 +4,14 @@ namespace ErsatzTV.Controllers.Api.Requests; public record ReplaceDecoTemplateRequest(string Name, List Items) { - public ReplaceDecoTemplateItems ToCommand(int decoTemplateGroupId, int decoTemplateId) => + public ReplaceDecoTemplateItems ToCommand( + int decoTemplateGroupId, + int decoTemplateId, + Option expectedVersion = default) => new( decoTemplateId, decoTemplateGroupId, Name, - (Items ?? []).Select(item => item.ToReplaceItem()).ToList()); + (Items ?? []).Select(item => item.ToReplaceItem()).ToList(), + expectedVersion); } diff --git a/ErsatzTV/Controllers/Api/Requests/ReplaceTemplateRequest.cs b/ErsatzTV/Controllers/Api/Requests/ReplaceTemplateRequest.cs index f2c5f20e0..330a76176 100644 --- a/ErsatzTV/Controllers/Api/Requests/ReplaceTemplateRequest.cs +++ b/ErsatzTV/Controllers/Api/Requests/ReplaceTemplateRequest.cs @@ -4,10 +4,11 @@ namespace ErsatzTV.Controllers.Api.Requests; public record ReplaceTemplateRequest(string Name, List Items) { - public ReplaceTemplateItems ToCommand(int templateGroupId, int templateId) => + public ReplaceTemplateItems ToCommand(int templateGroupId, int templateId, Option expectedVersion = default) => new( templateGroupId, templateId, Name, - (Items ?? []).Select(item => item.ToReplaceItem()).ToList()); + (Items ?? []).Select(item => item.ToReplaceItem()).ToList(), + expectedVersion); } diff --git a/ErsatzTV/Controllers/Api/TemplateController.cs b/ErsatzTV/Controllers/Api/TemplateController.cs index e02f87209..624320e38 100644 --- a/ErsatzTV/Controllers/Api/TemplateController.cs +++ b/ErsatzTV/Controllers/Api/TemplateController.cs @@ -134,6 +134,9 @@ public class TemplateController(IMediator mediator) : ControllerBase [HttpGet("/api/templates/{id:int}/items")] [Tags("Templates")] [EndpointSummary("Get template items")] + [EndpointDescription( + "Returns the template's items and a strong ETag of the template's version. Pass that ETag back as " + + "If-Match on the replace (PUT) to detect a concurrent edit (issue #253).")] [EndpointGroupName("general")] [ProducesResponseType(typeof(List), StatusCodes.Status200OK)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] @@ -145,6 +148,9 @@ public class TemplateController(IMediator mediator) : ControllerBase return ApiResults.NotFoundProblem(); } + // The items GET returns children, not the root, so read the template's version for the ETag. + ConcurrencyHeaders.SetETag(Response, template.Map(t => t.Version).IfNone(0)); + List items = await mediator.Send(new GetTemplateItems(id), cancellationToken); return new OkObjectResult( items.OrderBy(i => i.StartTime.TimeOfDay).Map(ProjectToResponseModel).ToList()); @@ -155,16 +161,32 @@ public class TemplateController(IMediator mediator) : ControllerBase [EndpointSummary("Replace a template and its items")] [EndpointDescription( "Replaces the template's name and its full item list. Each item assigns a block to a start time of day; " + - "items must not overlap (an item's end time is its start time plus the assigned block's duration).")] + "items must not overlap (an item's end time is its start time plus the assigned block's duration). " + + "Send the ETag from the items GET as If-Match to reject a stale overwrite with 412 (issue #253); " + + "a successful response carries the new ETag.")] [EndpointGroupName("general")] [ProducesResponseType(typeof(TemplateWithItemsResponseModel), StatusCodes.Status200OK)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status400BadRequest)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status412PreconditionFailed)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task Replace( int id, [Required] [FromBody] ReplaceTemplateRequest request, CancellationToken cancellationToken) { + IfMatchCondition ifMatch = ConcurrencyHeaders.ParseIfMatch(Request); + if (ifMatch.Kind is IfMatchKind.Malformed) + { + return new BadRequestObjectResult( + new ProblemDetails + { + Status = StatusCodes.Status400BadRequest, + Title = "Invalid If-Match header", + Detail = "If-Match must be a strong ETag of the resource version (e.g. \"3\") or \"*\"." + }); + } + Option maybeTemplate = await mediator.Send(new GetTemplateById(id), cancellationToken); if (maybeTemplate.IsNone) { @@ -174,16 +196,20 @@ public class TemplateController(IMediator mediator) : ControllerBase int templateGroupId = maybeTemplate.Map(t => t.TemplateGroupId).IfNone(0); Either> result = - await mediator.Send(request.ToCommand(templateGroupId, id), cancellationToken); + await mediator.Send(request.ToCommand(templateGroupId, id, ifMatch.ExpectedVersion), cancellationToken); return await result.Match( Left: error => Task.FromResult(error.ToErrorResult()), - Right: async _ => + Right: async items => { Option refreshed = await mediator.Send(new GetTemplateById(id), cancellationToken); - List items = await mediator.Send(new GetTemplateItems(id), cancellationToken); return refreshed.Match( - Some: vm => (IActionResult)new OkObjectResult(ProjectToWithItemsResponseModel(vm, items)), + Some: vm => + { + // Return the new ETag so a same-tab second save doesn't 412 against its own write. + ConcurrencyHeaders.SetETag(Response, vm.Version); + return (IActionResult)new OkObjectResult(ProjectToWithItemsResponseModel(vm, items)); + }, None: () => ApiResults.NotFoundProblem()); }); } diff --git a/web/src/api/decoTemplates.ts b/web/src/api/decoTemplates.ts index 3e88a15aa..09d01739a 100644 --- a/web/src/api/decoTemplates.ts +++ b/web/src/api/decoTemplates.ts @@ -1,4 +1,4 @@ -import { ApiError, request } from './client'; +import { ApiError, request, requestWithMeta, type ResponseWithMeta } from './client'; import type { components } from './generated/v1'; export type DecoTemplateGroup = components['schemas']['DecoTemplateGroupResponseModel']; @@ -46,8 +46,25 @@ export function getDecoTemplateItems(id: number): Promise { return request(`/api/deco-templates/${id}/items`); } -export function replaceDecoTemplate(id: number, body: ReplaceDecoTemplateRequest): Promise { - return request(`/api/deco-templates/${id}`, { body, method: 'PUT' }); +/** Load deco template items together with the deco template's concurrency ETag (issue #253). */ +export function getDecoTemplateItemsWithMeta(id: number): Promise> { + return requestWithMeta(`/api/deco-templates/${id}/items`); +} + +/** + * Replace a deco template. Pass the last-seen ETag as `If-Match` to reject a stale overwrite with + * 412; the resolved value carries the new ETag for a subsequent save (issue #253). + */ +export function replaceDecoTemplate( + id: number, + body: ReplaceDecoTemplateRequest, + ifMatch?: string | null +): Promise> { + return requestWithMeta(`/api/deco-templates/${id}`, { + body, + method: 'PUT', + headers: ifMatch ? { 'If-Match': ifMatch } : undefined + }); } export function messageFromDecoTemplateError(error: unknown, fallback = 'Unable to load deco templates'): string { diff --git a/web/src/api/templates.ts b/web/src/api/templates.ts index d4788d03f..c91c28e9d 100644 --- a/web/src/api/templates.ts +++ b/web/src/api/templates.ts @@ -1,4 +1,4 @@ -import { ApiError, request } from './client'; +import { ApiError, request, requestWithMeta, type ResponseWithMeta } from './client'; import type { components } from './generated/v1'; export type TemplateGroup = components['schemas']['TemplateGroupResponseModel']; @@ -47,8 +47,25 @@ export function getTemplateItems(id: number): Promise { return request(`/api/templates/${id}/items`); } -export function replaceTemplate(id: number, body: ReplaceTemplateRequest): Promise { - return request(`/api/templates/${id}`, { body, method: 'PUT' }); +/** Load template items together with the template's concurrency ETag (issue #253). */ +export function getTemplateItemsWithMeta(id: number): Promise> { + return requestWithMeta(`/api/templates/${id}/items`); +} + +/** + * Replace a template. Pass the last-seen ETag as `If-Match` to reject a stale overwrite with 412; + * the resolved value carries the new ETag for a subsequent save (issue #253). + */ +export function replaceTemplate( + id: number, + body: ReplaceTemplateRequest, + ifMatch?: string | null +): Promise> { + return requestWithMeta(`/api/templates/${id}`, { + body, + method: 'PUT', + headers: ifMatch ? { 'If-Match': ifMatch } : undefined + }); } export function copyTemplate(id: number, body: CopyTemplateRequest): Promise