Merge pull request '#253 PR1 — optimistic-concurrency contract (infra + Block reference)' (#263)
Build ErsatzTV Image / Docs update reminder (push) Has been skipped
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m15s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 10m38s
Build ErsatzTV Image / Build & push image (amd64) (push) Has been cancelled

This commit was merged in pull request #263.
This commit is contained in:
2026-07-11 16:06:11 +00:00
52 changed files with 15581 additions and 50 deletions
@@ -0,0 +1,33 @@
using ErsatzTV.Core;
using ErsatzTV.Core.Errors;
using Microsoft.EntityFrameworkCore;
namespace ErsatzTV.Application;
public static class ConcurrencyExtensions
{
/// <summary>
/// Persist pending changes, mapping the EF optimistic-concurrency failure to
/// <see cref="PreconditionFailedError" /> (→ 412). When a versioned root carries an
/// <c>IsConcurrencyToken</c> column and its <c>Version</c> is bumped before saving, EF emits
/// <c>UPDATE … WHERE Id=@id AND Version=@original</c>; a zero-row result (another writer won
/// the race between our load and save) throws <see cref="DbUpdateConcurrencyException" />.
/// This is the backstop that closes the load→save TOCTOU the handler pre-check cannot.
/// Issue #253.
/// </summary>
public static async Task<Either<BaseError, Unit>> SaveChangesWithConcurrencyGuard(
this DbContext dbContext,
CancellationToken cancellationToken)
{
try
{
await dbContext.SaveChangesAsync(cancellationToken);
return Unit.Default;
}
catch (DbUpdateConcurrencyException)
{
return new PreconditionFailedError(
"The resource was modified by another request. Reload and try again.");
}
}
}
@@ -8,4 +8,5 @@ public record BlockViewModel(
string GroupName,
string Name,
int Minutes,
BlockStopScheduling StopScheduling);
BlockStopScheduling StopScheduling,
int Version);
@@ -9,5 +9,6 @@ public record ReplaceBlockItems(
string Name,
int Minutes,
BlockStopScheduling StopScheduling,
List<ReplaceBlockItem> Items)
List<ReplaceBlockItem> Items,
Option<int> ExpectedVersion = default)
: IRequest<Either<BaseError, Unit>>;
@@ -16,10 +16,21 @@ public class ReplaceBlockItemsHandler(IDbContextFactory<TvContext> dbContextFact
{
await using TvContext dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken);
Validation<BaseError, Block> 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).
// LanguageExtensions.ToEither joins the Seq<BaseError> to a single BaseError (the native
// Validation.ToEither() would keep Seq and is called explicitly here to avoid that shadow).
Either<BaseError, Block> validated = LanguageExtensions.ToEither(validation)
.Bind(block => block.CheckVersion(request.ExpectedVersion));
return await validated.Match(
Right: block => Persist(dbContext, request, block, cancellationToken),
Left: error => Task.FromResult<Either<BaseError, Unit>>(error));
}
private static async Task<Unit> Persist(
private static async Task<Either<BaseError, Unit>> Persist(
TvContext dbContext,
ReplaceBlockItems request,
Block block,
@@ -33,7 +44,10 @@ public class ReplaceBlockItemsHandler(IDbContextFactory<TvContext> dbContextFact
dbContext.RemoveRange(block.Items);
block.Items = request.Items.Map(i => BuildItem(block, i.Index, 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).
block.Version++;
// TODO: refresh any playouts that use this schedule
// foreach (Playout playout in programSchedule.Playouts)
@@ -41,7 +55,9 @@ public class ReplaceBlockItemsHandler(IDbContextFactory<TvContext> dbContextFact
// await _channel.WriteAsync(new BuildPlayout(playout.Id, PlayoutBuildMode.Refresh));
// }
return Unit.Default;
// 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.
return await dbContext.SaveChangesWithConcurrencyGuard(cancellationToken);
}
private static BlockItem BuildItem(Block block, int index, ReplaceBlockItem item)
+1 -1
View File
@@ -32,7 +32,7 @@ internal static class Mapper
new(blockGroup.Id, blockGroup.Name);
internal static BlockViewModel ProjectToViewModel(Block block) =>
new(block.Id, block.BlockGroupId, block.BlockGroup.Name, block.Name, block.Minutes, block.StopScheduling);
new(block.Id, block.BlockGroupId, block.BlockGroup.Name, block.Name, block.Minutes, block.StopScheduling, block.Version);
internal static BlockItemViewModel ProjectToViewModel(BlockItem blockItem) =>
new(
@@ -3,9 +3,10 @@
namespace ErsatzTV.Core.Domain;
[SuppressMessage("Naming", "CA1711:Identifiers should not have incorrect suffix")]
public class Collection
public class Collection : IVersionedAggregate
{
public int Id { get; set; }
public int Version { get; set; }
public string Name { get; set; }
public bool UseCustomPlaybackOrder { get; set; }
public List<MediaItem> MediaItems { get; set; }
@@ -3,9 +3,10 @@
namespace ErsatzTV.Core.Domain;
[SuppressMessage("Naming", "CA1711:Identifiers should not have incorrect suffix")]
public class MultiCollection
public class MultiCollection : IVersionedAggregate
{
public int Id { get; set; }
public int Version { get; set; }
public string Name { get; set; }
public List<Collection> Collections { get; set; }
public List<SmartCollection> SmartCollections { get; set; }
+2 -1
View File
@@ -1,8 +1,9 @@
namespace ErsatzTV.Core.Domain;
public class Playlist
public class Playlist : IVersionedAggregate
{
public int Id { get; set; }
public int Version { get; set; }
public int PlaylistGroupId { get; set; }
public PlaylistGroup PlaylistGroup { get; set; }
public string Name { get; set; }
@@ -3,9 +3,10 @@
namespace ErsatzTV.Core.Domain;
[SuppressMessage("Naming", "CA1711:Identifiers should not have incorrect suffix")]
public class RerunCollection
public class RerunCollection : IVersionedAggregate
{
public int Id { get; set; }
public int Version { get; set; }
public string Name { get; set; }
public CollectionType CollectionType { get; set; }
public int? CollectionId { get; set; }
@@ -0,0 +1,12 @@
namespace ErsatzTV.Core.Domain;
/// <summary>
/// A root aggregate that carries an integer optimistic-concurrency token (issue #253).
/// The token is exposed as a strong <c>ETag</c> on the aggregate's GET, checked against a
/// PUT's <c>If-Match</c> header (mismatch → 412 Precondition Failed), and bumped on every
/// successful write so a stale second writer is rejected and every other client's ETag rotates.
/// </summary>
public interface IVersionedAggregate
{
int Version { get; set; }
}
+2 -1
View File
@@ -2,9 +2,10 @@
namespace ErsatzTV.Core.Domain;
public class Playout
public class Playout : IVersionedAggregate
{
public int Id { get; set; }
public int Version { get; set; }
public int ChannelId { get; set; }
public Channel Channel { get; set; }
public int? ProgramScheduleId { get; set; }
+2 -1
View File
@@ -2,9 +2,10 @@
namespace ErsatzTV.Core.Domain;
public class ProgramSchedule
public class ProgramSchedule : IVersionedAggregate
{
public int Id { get; set; }
public int Version { get; set; }
public string Name { get; set; }
public bool KeepMultiPartEpisodesTogether { get; set; }
public bool TreatCollectionsAsShows { get; set; }
+2 -1
View File
@@ -1,8 +1,9 @@
namespace ErsatzTV.Core.Domain.Scheduling;
public class Block
public class Block : IVersionedAggregate
{
public int Id { get; set; }
public int Version { get; set; }
public int BlockGroupId { get; set; }
public BlockGroup BlockGroup { get; set; }
public string Name { get; set; }
@@ -1,8 +1,9 @@
namespace ErsatzTV.Core.Domain.Scheduling;
public class DecoTemplate
public class DecoTemplate : IVersionedAggregate
{
public int Id { get; set; }
public int Version { get; set; }
public int DecoTemplateGroupId { get; set; }
public DecoTemplateGroup DecoTemplateGroup { get; set; }
public string Name { get; set; }
+2 -1
View File
@@ -1,8 +1,9 @@
namespace ErsatzTV.Core.Domain.Scheduling;
public class Template
public class Template : IVersionedAggregate
{
public int Id { get; set; }
public int Version { get; set; }
public int TemplateGroupId { get; set; }
public TemplateGroup TemplateGroup { get; set; }
public string Name { get; set; }
@@ -0,0 +1,14 @@
namespace ErsatzTV.Core.Errors;
/// <summary>
/// A <see cref="BaseError" /> raised when an optimistic-concurrency precondition fails — the
/// caller's <c>If-Match</c> version no longer matches the stored aggregate <c>Version</c>
/// (issue #253). REST endpoints map this to HTTP 412 Precondition Failed; it is distinct from
/// the 409 Conflict raised when a mutation races a background build lock (api-conventions §3a).
/// </summary>
public class PreconditionFailedError : BaseError
{
public PreconditionFailedError(string value) : base(value)
{
}
}
@@ -0,0 +1,24 @@
using ErsatzTV.Core.Domain;
using ErsatzTV.Core.Errors;
namespace ErsatzTV.Core;
public static class VersionedAggregateExtensions
{
/// <summary>
/// Optimistic-concurrency pre-check (issue #253). When the caller supplied an expected
/// version (from <c>If-Match</c>), fail with <see cref="PreconditionFailedError" /> (→ 412)
/// if it no longer matches the loaded aggregate. An absent expectation is a force-write
/// (Phase 1 back-compat). This returns a standalone <see cref="Either{L,R}" /> so the 412
/// is introduced AFTER the validation pipeline and never flattened to 422 by
/// <see cref="LanguageExtensions.Apply{T,TR}" /> (see api-conventions §7a).
/// </summary>
public static Either<BaseError, T> CheckVersion<T>(this T aggregate, Option<int> expectedVersion)
where T : IVersionedAggregate =>
expectedVersion.Match(
Some: expected => aggregate.Version == expected
? Right<BaseError, T>(aggregate)
: Left<BaseError, T>(new PreconditionFailedError(
"The resource was modified by another request. Reload and try again.")),
None: () => Right<BaseError, T>(aggregate));
}
File diff suppressed because it is too large Load Diff
@@ -0,0 +1,117 @@
using Microsoft.EntityFrameworkCore.Migrations;
#nullable disable
namespace ErsatzTV.Infrastructure.MySql.Migrations
{
/// <inheritdoc />
public partial class AddAggregateVersions : Migration
{
/// <inheritdoc />
protected override void Up(MigrationBuilder migrationBuilder)
{
migrationBuilder.AddColumn<int>(
name: "Version",
table: "Template",
type: "int",
nullable: false,
defaultValue: 0);
migrationBuilder.AddColumn<int>(
name: "Version",
table: "RerunCollection",
type: "int",
nullable: false,
defaultValue: 0);
migrationBuilder.AddColumn<int>(
name: "Version",
table: "ProgramSchedule",
type: "int",
nullable: false,
defaultValue: 0);
migrationBuilder.AddColumn<int>(
name: "Version",
table: "Playout",
type: "int",
nullable: false,
defaultValue: 0);
migrationBuilder.AddColumn<int>(
name: "Version",
table: "Playlist",
type: "int",
nullable: false,
defaultValue: 0);
migrationBuilder.AddColumn<int>(
name: "Version",
table: "MultiCollection",
type: "int",
nullable: false,
defaultValue: 0);
migrationBuilder.AddColumn<int>(
name: "Version",
table: "DecoTemplate",
type: "int",
nullable: false,
defaultValue: 0);
migrationBuilder.AddColumn<int>(
name: "Version",
table: "Collection",
type: "int",
nullable: false,
defaultValue: 0);
migrationBuilder.AddColumn<int>(
name: "Version",
table: "Block",
type: "int",
nullable: false,
defaultValue: 0);
}
/// <inheritdoc />
protected override void Down(MigrationBuilder migrationBuilder)
{
migrationBuilder.DropColumn(
name: "Version",
table: "Template");
migrationBuilder.DropColumn(
name: "Version",
table: "RerunCollection");
migrationBuilder.DropColumn(
name: "Version",
table: "ProgramSchedule");
migrationBuilder.DropColumn(
name: "Version",
table: "Playout");
migrationBuilder.DropColumn(
name: "Version",
table: "Playlist");
migrationBuilder.DropColumn(
name: "Version",
table: "MultiCollection");
migrationBuilder.DropColumn(
name: "Version",
table: "DecoTemplate");
migrationBuilder.DropColumn(
name: "Version",
table: "Collection");
migrationBuilder.DropColumn(
name: "Version",
table: "Block");
}
}
}
@@ -598,6 +598,10 @@ namespace ErsatzTV.Infrastructure.MySql.Migrations
b.Property<bool>("UseCustomPlaybackOrder")
.HasColumnType("tinyint(1)");
b.Property<int>("Version")
.IsConcurrencyToken()
.HasColumnType("int");
b.HasKey("Id");
b.HasIndex("Name")
@@ -1776,6 +1780,10 @@ namespace ErsatzTV.Infrastructure.MySql.Migrations
.HasColumnType("varchar(50)")
.UseCollation("utf8mb4_general_ci");
b.Property<int>("Version")
.IsConcurrencyToken()
.HasColumnType("int");
b.HasKey("Id");
b.HasIndex("Name")
@@ -1971,6 +1979,10 @@ namespace ErsatzTV.Infrastructure.MySql.Migrations
b.Property<int>("PlaylistGroupId")
.HasColumnType("int");
b.Property<int>("Version")
.IsConcurrencyToken()
.HasColumnType("int");
b.HasKey("Id");
b.HasIndex("PlaylistGroupId", "Name")
@@ -2089,6 +2101,10 @@ namespace ErsatzTV.Infrastructure.MySql.Migrations
b.Property<int>("Seed")
.HasColumnType("int");
b.Property<int>("Version")
.IsConcurrencyToken()
.HasColumnType("int");
b.HasKey("Id");
b.HasIndex("ChannelId");
@@ -2443,6 +2459,10 @@ namespace ErsatzTV.Infrastructure.MySql.Migrations
b.Property<bool>("TreatCollectionsAsShows")
.HasColumnType("tinyint(1)");
b.Property<int>("Version")
.IsConcurrencyToken()
.HasColumnType("int");
b.HasKey("Id");
b.HasIndex("Name")
@@ -2751,6 +2771,10 @@ namespace ErsatzTV.Infrastructure.MySql.Migrations
b.Property<int?>("SmartCollectionId")
.HasColumnType("int");
b.Property<int>("Version")
.IsConcurrencyToken()
.HasColumnType("int");
b.HasKey("Id");
b.HasIndex("CollectionId");
@@ -2819,6 +2843,10 @@ namespace ErsatzTV.Infrastructure.MySql.Migrations
b.Property<int>("StopScheduling")
.HasColumnType("int");
b.Property<int>("Version")
.IsConcurrencyToken()
.HasColumnType("int");
b.HasKey("Id");
b.HasIndex("BlockGroupId", "Name")
@@ -3090,6 +3118,10 @@ namespace ErsatzTV.Infrastructure.MySql.Migrations
.HasColumnType("varchar(50)")
.UseCollation("utf8mb4_general_ci");
b.Property<int>("Version")
.IsConcurrencyToken()
.HasColumnType("int");
b.HasKey("Id");
b.HasIndex("DecoTemplateGroupId", "Name")
@@ -3309,6 +3341,10 @@ namespace ErsatzTV.Infrastructure.MySql.Migrations
b.Property<int>("TemplateGroupId")
.HasColumnType("int");
b.Property<int>("Version")
.IsConcurrencyToken()
.HasColumnType("int");
b.HasKey("Id");
b.HasIndex("TemplateGroupId", "Name")
File diff suppressed because it is too large Load Diff
@@ -0,0 +1,117 @@
using Microsoft.EntityFrameworkCore.Migrations;
#nullable disable
namespace ErsatzTV.Infrastructure.Sqlite.Migrations
{
/// <inheritdoc />
public partial class AddAggregateVersions : Migration
{
/// <inheritdoc />
protected override void Up(MigrationBuilder migrationBuilder)
{
migrationBuilder.AddColumn<int>(
name: "Version",
table: "Template",
type: "INTEGER",
nullable: false,
defaultValue: 0);
migrationBuilder.AddColumn<int>(
name: "Version",
table: "RerunCollection",
type: "INTEGER",
nullable: false,
defaultValue: 0);
migrationBuilder.AddColumn<int>(
name: "Version",
table: "ProgramSchedule",
type: "INTEGER",
nullable: false,
defaultValue: 0);
migrationBuilder.AddColumn<int>(
name: "Version",
table: "Playout",
type: "INTEGER",
nullable: false,
defaultValue: 0);
migrationBuilder.AddColumn<int>(
name: "Version",
table: "Playlist",
type: "INTEGER",
nullable: false,
defaultValue: 0);
migrationBuilder.AddColumn<int>(
name: "Version",
table: "MultiCollection",
type: "INTEGER",
nullable: false,
defaultValue: 0);
migrationBuilder.AddColumn<int>(
name: "Version",
table: "DecoTemplate",
type: "INTEGER",
nullable: false,
defaultValue: 0);
migrationBuilder.AddColumn<int>(
name: "Version",
table: "Collection",
type: "INTEGER",
nullable: false,
defaultValue: 0);
migrationBuilder.AddColumn<int>(
name: "Version",
table: "Block",
type: "INTEGER",
nullable: false,
defaultValue: 0);
}
/// <inheritdoc />
protected override void Down(MigrationBuilder migrationBuilder)
{
migrationBuilder.DropColumn(
name: "Version",
table: "Template");
migrationBuilder.DropColumn(
name: "Version",
table: "RerunCollection");
migrationBuilder.DropColumn(
name: "Version",
table: "ProgramSchedule");
migrationBuilder.DropColumn(
name: "Version",
table: "Playout");
migrationBuilder.DropColumn(
name: "Version",
table: "Playlist");
migrationBuilder.DropColumn(
name: "Version",
table: "MultiCollection");
migrationBuilder.DropColumn(
name: "Version",
table: "DecoTemplate");
migrationBuilder.DropColumn(
name: "Version",
table: "Collection");
migrationBuilder.DropColumn(
name: "Version",
table: "Block");
}
}
}
@@ -579,6 +579,10 @@ namespace ErsatzTV.Infrastructure.Sqlite.Migrations
b.Property<bool>("UseCustomPlaybackOrder")
.HasColumnType("INTEGER");
b.Property<int>("Version")
.IsConcurrencyToken()
.HasColumnType("INTEGER");
b.HasKey("Id");
b.HasIndex("Name")
@@ -1699,6 +1703,10 @@ namespace ErsatzTV.Infrastructure.Sqlite.Migrations
.HasColumnType("varchar(50)")
.UseCollation("NOCASE");
b.Property<int>("Version")
.IsConcurrencyToken()
.HasColumnType("INTEGER");
b.HasKey("Id");
b.HasIndex("Name")
@@ -1886,6 +1894,10 @@ namespace ErsatzTV.Infrastructure.Sqlite.Migrations
b.Property<int>("PlaylistGroupId")
.HasColumnType("INTEGER");
b.Property<int>("Version")
.IsConcurrencyToken()
.HasColumnType("INTEGER");
b.HasKey("Id");
b.HasIndex("PlaylistGroupId", "Name")
@@ -1998,6 +2010,10 @@ namespace ErsatzTV.Infrastructure.Sqlite.Migrations
b.Property<int>("Seed")
.HasColumnType("INTEGER");
b.Property<int>("Version")
.IsConcurrencyToken()
.HasColumnType("INTEGER");
b.HasKey("Id");
b.HasIndex("ChannelId");
@@ -2336,6 +2352,10 @@ namespace ErsatzTV.Infrastructure.Sqlite.Migrations
b.Property<bool>("TreatCollectionsAsShows")
.HasColumnType("INTEGER");
b.Property<int>("Version")
.IsConcurrencyToken()
.HasColumnType("INTEGER");
b.HasKey("Id");
b.HasIndex("Name")
@@ -2636,6 +2656,10 @@ namespace ErsatzTV.Infrastructure.Sqlite.Migrations
b.Property<int?>("SmartCollectionId")
.HasColumnType("INTEGER");
b.Property<int>("Version")
.IsConcurrencyToken()
.HasColumnType("INTEGER");
b.HasKey("Id");
b.HasIndex("CollectionId");
@@ -2700,6 +2724,10 @@ namespace ErsatzTV.Infrastructure.Sqlite.Migrations
b.Property<int>("StopScheduling")
.HasColumnType("INTEGER");
b.Property<int>("Version")
.IsConcurrencyToken()
.HasColumnType("INTEGER");
b.HasKey("Id");
b.HasIndex("BlockGroupId", "Name")
@@ -2959,6 +2987,10 @@ namespace ErsatzTV.Infrastructure.Sqlite.Migrations
.HasColumnType("varchar(50)")
.UseCollation("NOCASE");
b.Property<int>("Version")
.IsConcurrencyToken()
.HasColumnType("INTEGER");
b.HasKey("Id");
b.HasIndex("DecoTemplateGroupId", "Name")
@@ -3166,6 +3198,10 @@ namespace ErsatzTV.Infrastructure.Sqlite.Migrations
b.Property<int>("TemplateGroupId")
.HasColumnType("INTEGER");
b.Property<int>("Version")
.IsConcurrencyToken()
.HasColumnType("INTEGER");
b.HasKey("Id");
b.HasIndex("TemplateGroupId", "Name")
@@ -10,6 +10,8 @@ public class CollectionConfiguration : IEntityTypeConfiguration<Collection>
{
builder.ToTable("Collection");
builder.Property(c => c.Version).IsConcurrencyToken();
builder.Property(c => c.Name)
.HasMaxLength(50)
.HasColumnType("varchar(50)");
@@ -10,6 +10,8 @@ public class MultiCollectionConfiguration : IEntityTypeConfiguration<MultiCollec
{
builder.ToTable("MultiCollection");
builder.Property(mc => mc.Version).IsConcurrencyToken();
builder.Property(mc => mc.Name)
.HasMaxLength(50)
.HasColumnType("varchar(50)");
@@ -10,6 +10,8 @@ public class PlaylistConfiguration : IEntityTypeConfiguration<Playlist>
{
builder.ToTable("Playlist");
builder.Property(p => p.Version).IsConcurrencyToken();
builder.Property(p => p.Name)
.HasMaxLength(50)
.HasColumnType("varchar(50)");
@@ -10,6 +10,8 @@ public class RerunCollectionConfiguration : IEntityTypeConfiguration<RerunCollec
{
builder.ToTable("RerunCollection");
builder.Property(rc => rc.Version).IsConcurrencyToken();
builder.Property(rc => rc.Name)
.HasMaxLength(50)
.HasColumnType("varchar(50)");
@@ -10,6 +10,8 @@ public class PlayoutConfiguration : IEntityTypeConfiguration<Playout>
{
builder.ToTable("Playout");
builder.Property(p => p.Version).IsConcurrencyToken();
builder.HasMany(p => p.ProgramScheduleAlternates)
.WithOne(a => a.Playout)
.HasForeignKey(a => a.PlayoutId)
@@ -10,6 +10,8 @@ public class ProgramScheduleConfiguration : IEntityTypeConfiguration<ProgramSche
{
builder.ToTable("ProgramSchedule");
builder.Property(p => p.Version).IsConcurrencyToken();
builder.Property(p => p.Name)
.HasMaxLength(50)
.HasColumnType("varchar(50)");
@@ -10,6 +10,8 @@ public class BlockConfiguration : IEntityTypeConfiguration<Block>
{
builder.ToTable("Block");
builder.Property(b => b.Version).IsConcurrencyToken();
builder.Property(b => b.Name)
.HasMaxLength(50)
.HasColumnType("varchar(50)");
@@ -10,6 +10,8 @@ public class DecoTemplateConfiguration : IEntityTypeConfiguration<DecoTemplate>
{
builder.ToTable("DecoTemplate");
builder.Property(d => d.Version).IsConcurrencyToken();
builder.Property(d => d.Name)
.HasMaxLength(50)
.HasColumnType("varchar(50)");
@@ -10,6 +10,8 @@ public class TemplateConfiguration : IEntityTypeConfiguration<Template>
{
builder.ToTable("Template");
builder.Property(t => t.Version).IsConcurrencyToken();
builder.Property(t => t.Name)
.HasMaxLength(50)
.HasColumnType("varchar(50)");
@@ -0,0 +1,157 @@
using ErsatzTV.Application;
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;
/// <summary>
/// Contract tests for the #253 optimistic-concurrency mechanic on the Block reference aggregate:
/// 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 <c>IsConcurrencyToken()</c> config on Block and the losing save silently succeeds instead of
/// mapping to a <see cref="PreconditionFailedError" />.
/// </summary>
[TestFixture]
public class ReplaceBlockItemsHandlerConcurrencyTests
{
private InMemoryTvContext _db = null!;
[SetUp]
public async Task SetUp() => _db = await InMemoryTvContext.CreateAsync();
[TearDown]
public async Task TearDown() => await _db.DisposeAsync();
private async Task SeedBlockAsync(int version)
{
await using TvContext ctx = _db.CreateContext();
ctx.Blocks.Add(
new Block
{
Id = 1,
BlockGroupId = 1,
Name = "Morning",
Minutes = 30,
StopScheduling = BlockStopScheduling.AfterDurationEnd,
Version = version,
Items = new List<BlockItem>()
});
await ctx.SaveChangesAsync();
}
private static ReplaceBlockItems Command(Option<int> expectedVersion) =>
new(
1,
1,
"Morning",
30,
BlockStopScheduling.AfterDurationEnd,
new List<ReplaceBlockItem>
{
new(0, CollectionType.SearchQuery, null, null, null, null, "News", "news", PlaybackOrder.Shuffle,
IncludeInProgramGuide: true, DisableWatermarks: false, [], [])
},
expectedVersion);
private async Task<int> ReadVersionAsync()
{
await using TvContext ctx = _db.CreateContext();
return await ctx.Blocks.Where(b => b.Id == 1).Select(b => b.Version).SingleAsync();
}
private static BaseError? LeftOrNull(Either<BaseError, Unit> result) =>
result.Match<BaseError?>(Right: _ => null, Left: e => e);
[Test]
public async Task Stale_If_Match_Should_Fail_Precondition_And_Not_Mutate()
{
await SeedBlockAsync(version: 2);
var handler = new ReplaceBlockItemsHandler(_db.Factory);
Either<BaseError, Unit> result = await handler.Handle(Command(Some(1)), CancellationToken.None);
LeftOrNull(result).ShouldBeOfType<PreconditionFailedError>();
// The pre-check runs before any mutation: version unchanged, no items written.
(await ReadVersionAsync()).ShouldBe(2);
await using TvContext ctx = _db.CreateContext();
(await ctx.BlockItems.CountAsync(i => i.BlockId == 1)).ShouldBe(0);
}
[Test]
public async Task Matching_If_Match_Should_Succeed_And_Bump_Version()
{
await SeedBlockAsync(version: 2);
var handler = new ReplaceBlockItemsHandler(_db.Factory);
Either<BaseError, Unit> 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 SeedBlockAsync(version: 2);
var handler = new ReplaceBlockItemsHandler(_db.Factory);
// None expected version = Phase-1 force-write regardless of the stored version.
Either<BaseError, Unit> 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 SeedBlockAsync(version: 5);
var handler = new ReplaceBlockItemsHandler(_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 SeedBlockAsync(version: 1);
// Two writers load the same version, then both bump-and-save. The IsConcurrencyToken guard on
// Block 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();
Block winner = await ctxWinner.Blocks.SingleAsync(b => b.Id == 1);
Block loser = await ctxLoser.Blocks.SingleAsync(b => b.Id == 1);
winner.Version++;
Either<BaseError, Unit> winnerResult = await ctxWinner.SaveChangesWithConcurrencyGuard(CancellationToken.None);
winnerResult.IsRight.ShouldBeTrue();
loser.Version++;
Either<BaseError, Unit> loserResult = await ctxLoser.SaveChangesWithConcurrencyGuard(CancellationToken.None);
LeftOrNull(loserResult).ShouldBeOfType<PreconditionFailedError>();
// Non-vacuous: the winner's write stuck at exactly its +1; the loser did not overwrite it.
(await ReadVersionAsync()).ShouldBe(2);
}
}
@@ -6,8 +6,10 @@ using ErsatzTV.Core;
using ErsatzTV.Core.Api.Scheduling;
using ErsatzTV.Core.Domain;
using ErsatzTV.Core.Domain.Scheduling;
using ErsatzTV.Core.Errors;
using LanguageExt;
using MediatR;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.Routing;
using NSubstitute;
@@ -28,7 +30,12 @@ public class BlockControllerTests
public void SetUp()
{
_mediator = Substitute.For<IMediator>();
_controller = new BlockController(_mediator);
_controller = new BlockController(_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]
@@ -258,6 +265,96 @@ public class BlockControllerTests
Arg.Any<CancellationToken>());
}
[Test]
public async Task GetItems_Should_Set_ETag_From_Block_Version()
{
_mediator.Send(Arg.Any<GetBlockById>(), Arg.Any<CancellationToken>())
.Returns(Option<BlockViewModel>.Some(MakeBlock(4, 2, "Morning", version: 9)));
_mediator.Send(Arg.Any<GetBlockItems>(), Arg.Any<CancellationToken>())
.Returns(new List<BlockItemViewModel>());
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 ReplaceBlockRequest("Morning", 60, BlockStopScheduling.AfterDurationEnd, []),
CancellationToken.None);
result.ShouldBeOfType<BadRequestObjectResult>();
await _mediator.DidNotReceive().Send(Arg.Any<GetBlockById>(), Arg.Any<CancellationToken>());
await _mediator.DidNotReceive().Send(Arg.Any<ReplaceBlockItems>(), Arg.Any<CancellationToken>());
}
[Test]
public async Task Replace_Should_Thread_If_Match_Version_Into_Command()
{
_controller.Request.Headers.IfMatch = "\"3\"";
_mediator.Send(Arg.Any<GetBlockById>(), Arg.Any<CancellationToken>())
.Returns(Option<BlockViewModel>.Some(MakeBlock(4, 7, "Morning", version: 4)));
_mediator.Send(Arg.Any<ReplaceBlockItems>(), Arg.Any<CancellationToken>())
.Returns(Right<BaseError, Unit>(Unit.Default));
_mediator.Send(Arg.Any<GetBlockItems>(), Arg.Any<CancellationToken>())
.Returns(new List<BlockItemViewModel>());
IActionResult result = await _controller.Replace(
4,
new ReplaceBlockRequest("Morning", 60, BlockStopScheduling.AfterDurationEnd, []),
CancellationToken.None);
result.ShouldBeOfType<OkObjectResult>();
// On success the response carries the refreshed block's ETag.
_controller.Response.Headers.ETag.ToString().ShouldBe("\"4\"");
await _mediator.Received(1).Send(
Arg.Is<ReplaceBlockItems>(c => c.ExpectedVersion == Option<int>.Some(3)),
Arg.Any<CancellationToken>());
}
[Test]
public async Task Replace_Without_If_Match_Should_Force_Write()
{
_mediator.Send(Arg.Any<GetBlockById>(), Arg.Any<CancellationToken>())
.Returns(Option<BlockViewModel>.Some(MakeBlock(4, 7, "Morning")));
_mediator.Send(Arg.Any<ReplaceBlockItems>(), Arg.Any<CancellationToken>())
.Returns(Right<BaseError, Unit>(Unit.Default));
_mediator.Send(Arg.Any<GetBlockItems>(), Arg.Any<CancellationToken>())
.Returns(new List<BlockItemViewModel>());
await _controller.Replace(
4,
new ReplaceBlockRequest("Morning", 60, BlockStopScheduling.AfterDurationEnd, []),
CancellationToken.None);
await _mediator.Received(1).Send(
Arg.Is<ReplaceBlockItems>(c => c.ExpectedVersion == Option<int>.None),
Arg.Any<CancellationToken>());
}
[Test]
public async Task Replace_Should_Return_412_On_Precondition_Failed()
{
_controller.Request.Headers.IfMatch = "\"2\"";
_mediator.Send(Arg.Any<GetBlockById>(), Arg.Any<CancellationToken>())
.Returns(Option<BlockViewModel>.Some(MakeBlock(4, 7, "Morning", version: 5)));
_mediator.Send(Arg.Any<ReplaceBlockItems>(), Arg.Any<CancellationToken>())
.Returns(Left<BaseError, Unit>(new PreconditionFailedError("stale")));
IActionResult result = await _controller.Replace(
4,
new ReplaceBlockRequest("Morning", 60, BlockStopScheduling.AfterDurationEnd, []),
CancellationToken.None);
var objectResult = result.ShouldBeOfType<ObjectResult>();
objectResult.StatusCode.ShouldBe(StatusCodes.Status412PreconditionFailed);
}
[Test]
public async Task Replace_Should_Return_404_When_Block_Missing()
{
@@ -380,8 +477,8 @@ public class BlockControllerTests
result.ShouldBeOfType<UnprocessableEntityObjectResult>();
}
private static BlockViewModel MakeBlock(int id, int groupId, string name) =>
new(id, groupId, "Group", name, 30, BlockStopScheduling.AfterDurationEnd);
private static BlockViewModel MakeBlock(int id, int groupId, string name, int version = 1) =>
new(id, groupId, "Group", name, 30, BlockStopScheduling.AfterDurationEnd, version);
private static BlockItemViewModel MakeItem(int id, int index) =>
new(
@@ -1147,7 +1147,7 @@ public class PlayoutControllerTests
_mediator.Send(Arg.Any<GetPlayoutById>(), Arg.Any<CancellationToken>())
.Returns(Option<PlayoutNameViewModel>.Some(MakePlayout(9) with { ScheduleKind = PlayoutScheduleKind.Block }));
_mediator.Send(Arg.Any<GetAllBlocksForPlayout>(), Arg.Any<CancellationToken>())
.Returns([new BlockViewModel(10, 1, "Morning", "Toons", 60, BlockStopScheduling.AfterDurationEnd)]);
.Returns([new BlockViewModel(10, 1, "Morning", "Toons", 60, BlockStopScheduling.AfterDurationEnd, 1)]);
IActionResult result = await _controller.GetBlocks(9, CancellationToken.None);
@@ -19,6 +19,13 @@ public class ApiResultsTests
result.ShouldBeOfType<NotFoundObjectResult>().StatusCode.ShouldBe(404);
}
[Test]
public void ToErrorResult_Should_Map_PreconditionFailedError_To_412()
{
IActionResult result = new PreconditionFailedError("stale").ToErrorResult();
result.ShouldBeOfType<ObjectResult>().StatusCode.ShouldBe(412);
}
[Test]
public void ToErrorResult_Should_Return_ProblemDetails_For_NotFoundError()
{
@@ -0,0 +1,70 @@
using ErsatzTV.Extensions;
using LanguageExt;
using Microsoft.AspNetCore.Http;
using NUnit.Framework;
using Shouldly;
using static LanguageExt.Prelude;
namespace ErsatzTV.Tests.Extensions;
[TestFixture]
public class ConcurrencyHeadersTests
{
private static IfMatchCondition Parse(string? ifMatch)
{
var context = new DefaultHttpContext();
if (ifMatch is not null)
{
context.Request.Headers.IfMatch = ifMatch;
}
return ConcurrencyHeaders.ParseIfMatch(context.Request);
}
[Test]
public void Absent_Header_Is_Absent()
{
IfMatchCondition result = Parse(null);
result.Kind.ShouldBe(IfMatchKind.Absent);
result.ExpectedVersion.ShouldBe(Option<int>.None);
}
[Test]
public void Wildcard_Is_Any_And_Forces_Write()
{
IfMatchCondition result = Parse("*");
result.Kind.ShouldBe(IfMatchKind.Any);
result.ExpectedVersion.ShouldBe(Option<int>.None);
}
[TestCase("\"0\"", 0)]
[TestCase("\"3\"", 3)]
[TestCase("\"2147483647\"", int.MaxValue)]
public void Canonical_Strong_Tag_Parses_To_Version(string header, int expected)
{
IfMatchCondition result = Parse(header);
result.Kind.ShouldBe(IfMatchKind.Version);
result.ExpectedVersion.ShouldBe(Some(expected));
}
// An ETag is opaque: only the exact canonical form we emit is accepted. Padded / signed /
// whitespaced / weak / unquoted / overflowing / list values are all rejected as malformed (→ 400),
// never silently coerced to a version that would match a stale write.
[TestCase("\"03\"")] // leading zero
[TestCase("\"+3\"")] // explicit sign
[TestCase("\"-3\"")] // negative
[TestCase("\" 3 \"")] // surrounding whitespace
[TestCase("\"3.0\"")] // non-integer
[TestCase("\"\"")] // empty tag
[TestCase("3")] // unquoted
[TestCase("W/\"3\"")] // weak tag
[TestCase("\"3\", \"5\"")] // tag list
[TestCase("\"99999999999999999999\"")] // overflows int
[TestCase("garbage")]
public void Non_Canonical_Values_Are_Malformed(string header)
{
IfMatchCondition result = Parse(header);
result.Kind.ShouldBe(IfMatchKind.Malformed);
result.ExpectedVersion.ShouldBe(Option<int>.None);
}
}
+30 -4
View File
@@ -128,6 +128,9 @@ public class BlockController(IMediator mediator) : ControllerBase
[HttpGet("/api/blocks/{id:int}/items")]
[Tags("Blocks")]
[EndpointSummary("Get block items")]
[EndpointDescription(
"Returns the block's items and a strong ETag of the block'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<BlockItemResponseModel>), StatusCodes.Status200OK)]
[ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)]
@@ -139,6 +142,9 @@ public class BlockController(IMediator mediator) : ControllerBase
return ApiResults.NotFoundProblem();
}
// The items GET returns children, not the root, so read the block's version for the ETag.
ConcurrencyHeaders.SetETag(Response, block.Map(b => b.Version).IfNone(0));
List<BlockItemViewModel> items = await mediator.Send(new GetBlockItems(id), cancellationToken);
return new OkObjectResult(items.OrderBy(i => i.Index).Map(ProjectToResponseModel).ToList());
}
@@ -148,16 +154,32 @@ public class BlockController(IMediator mediator) : ControllerBase
[EndpointSummary("Replace a block and its items")]
[EndpointDescription(
"Replaces the block's name/minutes/stop-scheduling and its full item list. Item indexes are assigned " +
"from the array order. Minutes must be greater than zero, divisible by 5, and at most 24 hours.")]
"from the array order. Minutes must be greater than zero, divisible by 5, and at most 24 hours. " +
"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(BlockWithItemsResponseModel), 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<IActionResult> Replace(
int id,
[Required] [FromBody] ReplaceBlockRequest 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<BlockViewModel> maybeBlock = await mediator.Send(new GetBlockById(id), cancellationToken);
if (maybeBlock.IsNone)
{
@@ -167,7 +189,7 @@ public class BlockController(IMediator mediator) : ControllerBase
int groupId = maybeBlock.Map(b => b.GroupId).IfNone(0);
Either<BaseError, Unit> result =
await mediator.Send(request.ToCommand(groupId, id), cancellationToken);
await mediator.Send(request.ToCommand(groupId, id, ifMatch.ExpectedVersion), cancellationToken);
return await result.Match(
Left: error => Task.FromResult(error.ToErrorResult()),
@@ -176,8 +198,12 @@ public class BlockController(IMediator mediator) : ControllerBase
Option<BlockViewModel> refreshed = await mediator.Send(new GetBlockById(id), cancellationToken);
List<BlockItemViewModel> items = await mediator.Send(new GetBlockItems(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());
});
}
@@ -9,12 +9,13 @@ public record ReplaceBlockRequest(
BlockStopScheduling StopScheduling,
List<BlockItemRequest> Items)
{
public ReplaceBlockItems ToCommand(int blockGroupId, int blockId) =>
public ReplaceBlockItems ToCommand(int blockGroupId, int blockId, Option<int> expectedVersion = default) =>
new(
blockGroupId,
blockId,
Name,
Minutes,
StopScheduling,
(Items ?? []).Select((item, index) => item.ToReplaceItem(index)).ToList());
(Items ?? []).Select((item, index) => item.ToReplaceItem(index)).ToList(),
expectedVersion);
}
+18 -4
View File
@@ -1,6 +1,7 @@
using System.Diagnostics.CodeAnalysis;
using ErsatzTV.Core;
using ErsatzTV.Core.Errors;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
namespace ErsatzTV.Extensions;
@@ -14,11 +15,24 @@ namespace ErsatzTV.Extensions;
[SuppressMessage("ReSharper", "VSTHRD003")]
public static class ApiResults
{
/// <summary>Maps a failure to 404 when it is a <see cref="NotFoundError" />, otherwise 422.</summary>
/// <summary>
/// Maps a failure to 404 when it is a <see cref="NotFoundError" />, 412 when it is a
/// <see cref="PreconditionFailedError" /> (optimistic-concurrency mismatch, issue #253),
/// otherwise 422.
/// </summary>
public static IActionResult ToErrorResult(this BaseError error) =>
error is NotFoundError
? new NotFoundObjectResult(CreateProblemDetails(404, "Resource not found", error.Value))
: new UnprocessableEntityObjectResult(CreateProblemDetails(422, "Validation failed", error.Value));
error switch
{
NotFoundError =>
new NotFoundObjectResult(CreateProblemDetails(404, "Resource not found", error.Value)),
PreconditionFailedError =>
new ObjectResult(CreateProblemDetails(412, "Precondition Failed", error.Value))
{
StatusCode = StatusCodes.Status412PreconditionFailed
},
_ =>
new UnprocessableEntityObjectResult(CreateProblemDetails(422, "Validation failed", error.Value))
};
/// <summary>Right: 201 Created with a Location header and body; Left: 404 (NotFound) or 422.</summary>
public static IActionResult ToCreatedResult<TR>(
+77
View File
@@ -0,0 +1,77 @@
using System.Globalization;
using LanguageExt;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Primitives;
namespace ErsatzTV.Extensions;
/// <summary>Classification of a request's <c>If-Match</c> header for the #253 concurrency contract.</summary>
public enum IfMatchKind
{
/// <summary>No <c>If-Match</c> header — Phase 1 force-write (Phase 2 will make this a 428).</summary>
Absent,
/// <summary><c>If-Match: *</c> — the scripted force-write escape hatch; skip the version check.</summary>
Any,
/// <summary>A strong entity-tag of a decimal aggregate version, e.g. <c>"3"</c>.</summary>
Version,
/// <summary>An unparseable value — the controller returns 400.</summary>
Malformed
}
public readonly record struct IfMatchCondition(IfMatchKind Kind, int Version)
{
/// <summary>The version to check against, or <c>None</c> for absent/wildcard (force-write).</summary>
public Option<int> ExpectedVersion => Kind == IfMatchKind.Version ? Version : Option<int>.None;
}
/// <summary>
/// Parse/emit the optimistic-concurrency HTTP headers (issue #253). GET responses carry a strong
/// <c>ETag</c> of the aggregate's integer <c>Version</c>; PUT requests carry the last-seen version
/// in <c>If-Match</c>. See <c>docs/api-conventions.md</c> §7a.
/// </summary>
public static class ConcurrencyHeaders
{
public static IfMatchCondition ParseIfMatch(HttpRequest request)
{
StringValues raw = request.Headers.IfMatch;
if (StringValues.IsNullOrEmpty(raw))
{
return new IfMatchCondition(IfMatchKind.Absent, 0);
}
string value = raw.ToString().Trim();
if (value == "*")
{
return new IfMatchCondition(IfMatchKind.Any, 0);
}
// Strong entity-tag of a decimal version, e.g. "3". Weak tags (W/"…") are not honored:
// this contract's ETags are always strong. An ETag is an opaque token, so only the exact
// canonical form we emit is accepted — a non-negative decimal with no sign, surrounding
// whitespace, or leading zeros (`NumberStyles.None` + the leading-zero guard reject "+3",
// " 3 ", and "03", which must NOT be treated as equal to the emitted "3").
if (value.Length >= 2 && value[0] == '"' && value[^1] == '"')
{
string inner = value[1..^1];
if (inner.Length > 0 && (inner.Length == 1 || inner[0] != '0') &&
int.TryParse(inner, NumberStyles.None, CultureInfo.InvariantCulture, out int version))
{
return new IfMatchCondition(IfMatchKind.Version, version);
}
}
// Everything else (a valid-but-non-canonical strong tag like "03", a weak tag W/"3", an
// entity-tag list, or plain garbage) is treated as Malformed → 400. Strictly, RFC 7232 would
// 412 a syntactically-valid tag that merely doesn't strong-match; that refinement (plus
// weak-tag comparison, list support, and 412-vs-404 ordering) is deferred to the #197 cold
// contract pass — see #265. This is fail-safe (the mutation is rejected, never applied) and the
// first-party SPA only ever echoes the single canonical tag we emit.
return new IfMatchCondition(IfMatchKind.Malformed, 0);
}
public static void SetETag(HttpResponse response, int version) =>
response.Headers.ETag = $"\"{version}\"";
}
+42 -1
View File
@@ -565,7 +565,7 @@
"Blocks"
],
"summary": "Replace a block and its items",
"description": "Replaces the block's name/minutes/stop-scheduling and its full item list. Item indexes are assigned from the array order. Minutes must be greater than zero, divisible by 5, and at most 24 hours.",
"description": "Replaces the block's name/minutes/stop-scheduling and its full item list. Item indexes are assigned from the array order. Minutes must be greater than zero, divisible by 5, and at most 24 hours. 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.",
"parameters": [
{
"name": "id",
@@ -623,6 +623,26 @@
}
}
},
"400": {
"description": "Bad Request",
"content": {
"text/plain": {
"schema": {
"$ref": "#/components/schemas/ProblemDetails"
}
},
"application/json": {
"schema": {
"$ref": "#/components/schemas/ProblemDetails"
}
},
"text/json": {
"schema": {
"$ref": "#/components/schemas/ProblemDetails"
}
}
}
},
"404": {
"description": "Not Found",
"content": {
@@ -643,6 +663,26 @@
}
}
},
"412": {
"description": "Precondition Failed",
"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": {
@@ -672,6 +712,7 @@
"Blocks"
],
"summary": "Get block items",
"description": "Returns the block's items and a strong ETag of the block's version. Pass that ETag back as If-Match on the replace (PUT) to detect a concurrent edit (issue #253).",
"parameters": [
{
"name": "id",
+55 -1
View File
@@ -84,7 +84,7 @@ hand-rolling `IActionResult` status codes:
| Method | Input | Output |
|---|---|---|
| `ToErrorResult()` | `BaseError` | 404 if `NotFoundError`, else 422 (`ProblemDetails`) |
| `ToErrorResult()` | `BaseError` | 404 if `NotFoundError`, **412 if `PreconditionFailedError`** (optimistic-concurrency mismatch, §7a), else 422 (`ProblemDetails`) |
| `ToCreatedResult(location, body)` | `Either<BaseError, T>` | `Left``ToErrorResult()`; `Right` → 201 + `Location` header |
| `ToUpdatedResult()` | `Either<BaseError, T>` | `Left``ToErrorResult()`; `Right` → 200 + body |
| `ToDeletedResult()` | `Either<BaseError, Unit>` | `Left``ToErrorResult()`; `Right` → 204 |
@@ -289,6 +289,60 @@ via `ProgramScheduleItemQueryExtensions.IncludeScheduleItemDetails()` (the one i
the NRE; the controller's `.ToList()`/serialization does (regression: `ScheduleItemWriteProjectionTests`).
GET handlers that feed an ordered list must also `.OrderBy(i => i.Index)` — id order is not index order.
## 7a. PUT-replace concurrency (ETag / If-Match / 412)
The replace-all aggregate PUTs (blocks, templates, schedules items, playlists, collections, playouts,
etc.) carry an **optimistic-concurrency contract** so a stale second tab can't silently overwrite a
fresher edit (issue #253). The Block endpoints are the reference implementation; PRs 24 fan the same
recipe across the other aggregates.
**Token.** Each versioned root implements `IVersionedAggregate` (`int Version`, EF-mapped with
`.IsConcurrencyToken()` in its `IEntityTypeConfiguration`). A single dual-provider migration
(`AddAggregateVersions`) adds the column (`nullable: false, defaultValue: 0`). Do **not** overload the
existing `DateUpdated` — a plain `int` is portable across SQLite/MySQL and decoupled from UI cosmetics.
**Transport.** The aggregate's GET (the one the editor loads from — e.g. `GET /api/blocks/{id}/items`)
emits a strong `ETag: "3"` of `Version`; the PUT sends it back as `If-Match: "3"`. Mismatch → **412
Precondition Failed** (distinct from the §3a **409** "build in progress" lock guard). A successful PUT
returns the **new** ETag (post-increment) so a same-tab second save doesn't 412 against its own write.
`If-Match: *` and (Phase 1) a missing header force-write; a non-canonical/weak/list/malformed header
→ 400 (fail-safe; the stricter RFC 7232 "valid-but-non-matching tag → 412" refinement is deferred to
#197 — see #265). Parse/emit with `ErsatzTV.Extensions.ConcurrencyHeaders` (`ParseIfMatch` → `IfMatchCondition.ExpectedVersion : Option<int>`,
`SetETag`). The items GET returns *children*, so the controller reads `root.Version` separately for the
header (here `BlockViewModel` carries `Version`, projected but **not** echoed in the response body —
header-only).
**Handler recipe (the error-prone part).** Introduce the concurrency check as a **standalone `Either`
AFTER** the validation pipeline, never via `Apply``LanguageExtensions.Apply`/`ToEither` `Join()` a
`Seq<BaseError>` down to a base `BaseError`, which would flatten `PreconditionFailedError` to a 422. The
reference shape (`ReplaceBlockItemsHandler`):
```csharp
Either<BaseError, Block> validated = LanguageExtensions.ToEither(validation) // explicit: the native
.Bind(block => block.CheckVersion(request.ExpectedVersion)); // Validation.ToEither() shadows ours
return await validated.Match(
Right: block => Persist(dbContext, request, block, cancellationToken),
Left: error => Task.FromResult<Either<BaseError, Unit>>(error));
```
In `Persist`, bump **unconditionally** before saving — `root.Version++` — because EF emits the root
UPDATE only when a scalar actually differs, so a same-value/no-op PUT-back would otherwise neither fire
the token nor rotate other clients' ETags. Then save through
`dbContext.SaveChangesWithConcurrencyGuard(ct)` (maps `DbUpdateConcurrencyException` → 412), which is the
backstop that closes the load→save TOCTOU the pre-check can't. `CheckVersion` (pure, on
`IVersionedAggregate`) and `SaveChangesWithConcurrencyGuard` live in `ErsatzTV.Core` /
`ErsatzTV.Application` respectively.
Add `[ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status400BadRequest)]` and `…412…` to the
PUT action. **Config-only boundary**: every mutating handler of an aggregate's *editor-visible config
state* bumps `Version` (incl. bulk `ExecuteUpdate/Delete` writers, which add
`.SetProperty(x => x.Version, x => x.Version + 1)`); regenerated build output (playout items/history) is
outside the token — its handlers neither bump nor are guarded. Test the reference with: stale-If-Match →
412 (no mutation), matching/absent → success + bump, no-op save still bumps, and a two-context racing
save → 412 (prove it non-vacuous by dropping `.IsConcurrencyToken()` and watching the race test fail).
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.
## 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
+33
View File
@@ -649,3 +649,36 @@ action of the Step 2 deletion PR merge** (not before — `main` moves until then
cut.
Not cut this session — `main` still carries Blazor and will advance before the removal PR.
## 2026-07-11 — Optimistic-concurrency contract for replace-all PUTs (#253 PR1: infra + Block reference)
Replace-all aggregate PUTs had **no** optimistic concurrency — a stale second tab silently overwrote a
fresher edit (200, no signal) across ~10 aggregate surfaces. PR1 lands the shared contract on the Block
reference aggregate; PRs 24 fan it out. The full ratified design + independent-review hardening is
[#253#issuecomment-8472](http://192.168.1.95:3000/timothy/ersatztv/issues/253#issuecomment-8472);
the mechanics live in `api-conventions.md` §7a. Decisions frozen here:
- **Token = uniform plain `int Version`** on each root implementing `IVersionedAggregate`, EF-mapped
`.IsConcurrencyToken()`, one dual-provider migration (`AddAggregateVersions`, `defaultValue: 0`). **Not**
a reused `DateUpdated` (tick-collision, SQLite TEXT precision, couples UI cosmetics to correctness) and
**not** a MySQL-native rowversion (portability over provider-native).
- **412 Precondition Failed**, not 409 — 409 stays the §3a EntityLocker "build in progress" guard;
distinct codes → distinct SPA UX. New `PreconditionFailedError : BaseError` → 412 in `ApiResults.ToErrorResult`.
- **Pre-check AND EF token both required.** The handler pre-check (a standalone `Either` introduced AFTER
the validation pipeline — never via `Apply`, which `Join()`-flattens the subtype to 422) gives a clean
412; the unconditional `root.Version++` + `IsConcurrencyToken` UPDATE-guard + a `SaveChangesWithConcurrencyGuard`
backstop closes the residual load→save TOCTOU (`DbUpdateConcurrencyException` → 412).
- **Unconditional bump** (not "only when a child changed"): EF writes the root row only when a scalar
differs, so a no-op PUT-back must still bump to fire the token and rotate every other client's ETag.
- **Config-only aggregate boundary**: every mutating handler of a root's *editor-visible config state*
bumps `Version` (incl. bulk `ExecuteUpdate/Delete` writers via `.SetProperty`); regenerated build output
(playout items/history) is outside the token — neither bumped nor guarded.
- **Header-only ETag**, strong tag of the decimal `Version`; parsed/emitted by `ConcurrencyHeaders`. The
successful PUT returns the new ETag (else a same-tab second save 412s against its own write).
- **Phasing**: Phase 1 (this arc) = a missing `If-Match` force-writes (zero breakage) while the SPA starts
echoing; Phase 2 (a later PR) flips missing → **428** after every editor echoes and one release soaks.
`If-Match: *` stays the scripted force-write escape hatch.
- **Child stable-identity is OUT of #253** (the "moved fill-group item inherits the wrong slot's state"
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**).
+1
View File
@@ -71,6 +71,7 @@ Channel (1) ──< Playout (0..N per channel; ChannelPlayoutSource distinguishe
| **Channel concepts** | `Number` (validated by `Channel.NumberValidator` regex), `Group`, `PlayoutSource` (Generated/Mirror; Mirror channels relay another channel via `MirrorSourceChannelId`+`PlayoutOffset`), `PlayoutMode` (Continuous/OnDemand), `TranscodeMode` (OnDemand only, today), `IdleBehavior` (StopOnDisconnect/KeepRunning), `StreamingMode` (TransportStream/HttpLiveStreamingDirect/HttpLiveStreamingSegmenter/TransportStreamHybrid). | `Channel` | `/app/channels`, `/app/edit-channel/{id}`, `/app/new-channel` |
| **Guide / EPG (XMLTV)** | Per-channel programme guide generated from playout items; channels with `ShowInEpg=false` are excluded. | `GetChannelGuideHandler` | `/app/guide` (viewer); settings at `/app/settings/xmltv` |
| **M3U** | The channel lineup playlist Jellyfin/Dispatcharr consume. | `ChannelPlaylist.ToM3U()` | — |
| **Aggregate `Version`** | Optimistic-concurrency token (issue #253): a plain `int Version` on the 9 replace-all roots — `ProgramSchedule`, `Block`, `Template`, `DecoTemplate`, `Playlist`, `Collection`, `Playout`, `MultiCollection`, `RerunCollection` — implementing `IVersionedAggregate`, EF-mapped `.IsConcurrencyToken()`. Surfaced as a strong ETag on the aggregate's GET and checked against `If-Match` on the PUT (mismatch → 412). See `api-conventions.md` §7a. | `IVersionedAggregate` | (not user-edited) |
## Where things are edited (SPA routes)
+21
View File
@@ -133,6 +133,27 @@ spec-cased vs runtime-cased key (the old `PlaybackTroubleshootingScreen` `#198`
`data.channel.fFmpegProfileId` has been removed — read `ffmpegProfileId` straight off the typed
response). When you mock an API response in a test, use the generated (runtime) casing.
## 4a. Optimistic-concurrency editors (ETag / If-Match / 412)
Replace-all editors (Blocks is the reference; see `api-conventions.md` §7a for the server contract,
issue #253) must round-trip the aggregate's concurrency ETag so a stale tab can't silently overwrite a
fresher edit:
- **Transport seam** (`web/src/api/client.ts`): `requestWithMeta<T>(path, options)` returns
`{ data, etag }` (reads the `ETag` response header). `request<T>` delegates to it and drops the meta —
keep using `request` for endpoints without a concurrency token.
- **Domain module** (`web/src/api/blocks.ts`): expose a `…WithMeta` load
(`getBlockItemsWithMeta``{ data, etag }`) and make the replace accept the last-seen ETag and return
the new one: `replaceBlock(id, body, ifMatch?)``requestWithMeta(..., { headers: ifMatch ? {'If-Match': ifMatch} : undefined })`.
- **Editor**: hold the ETag in a `useRef`; set it from the load GET, and **replace it from the PUT
response's ETag on every successful save** (a same-tab second save otherwise 412s against its own
write). Keyed reload: a `reloadKey` state in the load `useEffect` dep array lets the conflict flow
re-fetch.
- **412 UX**: catch `error instanceof ApiError && error.status === 412` on save and open a blocking
"changed elsewhere — reload (unsaved changes discarded)" `ConfirmDialog` (Reload bumps `reloadKey`),
distinct from a 409 ("build in progress — retry shortly"). All other errors stay the generic save-error
path. Reference: `web/src/screens/BlocksScreen.tsx` `BlockEditor`.
## 5. Artwork rendering
Render `item.artwork` / `item.poster` (or whatever the DTO field is named) **directly as an `<img
+39
View File
@@ -8,6 +8,7 @@ import {
getBlock,
getBlockGroups,
getBlockItems,
getBlockItemsWithMeta,
getBlocks,
previewBlock,
replaceBlock,
@@ -119,6 +120,44 @@ describe('blocks api client', () => {
expect(body.items).toHaveLength(1);
});
it('getBlockItemsWithMeta returns the items and the ETag', async () => {
vi.spyOn(window, 'fetch').mockResolvedValue(
new Response(JSON.stringify([{ id: 1 }]), {
headers: { ETag: '"5"', 'Content-Type': 'application/json' },
status: 200
})
);
const result = await getBlockItemsWithMeta(4);
expect(result.etag).toBe('"5"');
expect(result.data).toHaveLength(1);
});
it('replaceBlock sends If-Match when an ETag is supplied and returns the new ETag', async () => {
const fetchMock = vi.spyOn(window, 'fetch').mockResolvedValue(
new Response(JSON.stringify({ id: 4, items: [] }), {
headers: { ETag: '"6"', 'Content-Type': 'application/json' },
status: 200
})
);
const result = await replaceBlock(4, sampleReplace, '"5"');
const [, init] = fetchMock.mock.calls[0];
expect(init?.headers).toMatchObject({ 'If-Match': '"5"' });
expect(result.etag).toBe('"6"');
});
it('replaceBlock omits If-Match when no ETag is supplied', async () => {
const fetchMock = vi.spyOn(window, 'fetch').mockResolvedValue(jsonResponse({ id: 4, items: [] }));
await replaceBlock(4, sampleReplace);
const [, init] = fetchMock.mock.calls[0];
expect(init?.headers).not.toHaveProperty('If-Match');
});
it('previewBlock POSTs to the preview route', async () => {
const fetchMock = vi.spyOn(window, 'fetch').mockResolvedValue(jsonResponse([]));
await previewBlock(4, sampleReplace);
+20 -3
View File
@@ -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 BlockGroup = components['schemas']['BlockGroupResponseModel'];
@@ -49,8 +49,25 @@ export function getBlockItems(id: number): Promise<BlockItem[]> {
return request<BlockItem[]>(`/api/blocks/${id}/items`);
}
export function replaceBlock(id: number, body: ReplaceBlockRequest): Promise<BlockWithItems> {
return request<BlockWithItems>(`/api/blocks/${id}`, { body, method: 'PUT' });
/** Load block items together with the block's concurrency ETag (issue #253). */
export function getBlockItemsWithMeta(id: number): Promise<ResponseWithMeta<BlockItem[]>> {
return requestWithMeta<BlockItem[]>(`/api/blocks/${id}/items`);
}
/**
* Replace a block. 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 replaceBlock(
id: number,
body: ReplaceBlockRequest,
ifMatch?: string | null
): Promise<ResponseWithMeta<BlockWithItems>> {
return requestWithMeta<BlockWithItems>(`/api/blocks/${id}`, {
body,
method: 'PUT',
headers: ifMatch ? { 'If-Match': ifMatch } : undefined
});
}
export function previewBlock(id: number, body: ReplaceBlockRequest): Promise<BlockPreviewItem[]> {
+37 -1
View File
@@ -1,6 +1,6 @@
import { beforeEach, describe, expect, it, vi } from 'vitest';
import { setStoredApiKey } from './auth';
import { ApiError, request } from './client';
import { ApiError, request, requestWithMeta } from './client';
describe('API request client', () => {
beforeEach(() => {
@@ -94,4 +94,40 @@ describe('API request client', () => {
})
);
});
it('requestWithMeta returns the response ETag alongside the parsed body', async () => {
vi.spyOn(window, 'fetch').mockResolvedValue(
new Response(JSON.stringify([{ id: 1 }]), {
headers: { ETag: '"7"', 'Content-Type': 'application/json' },
status: 200
})
);
const result = await requestWithMeta<{ id: number }[]>('/api/blocks/1/items');
expect(result.etag).toBe('"7"');
expect(result.data).toEqual([{ id: 1 }]);
});
it('requestWithMeta returns a null ETag when the response has none', async () => {
vi.spyOn(window, 'fetch').mockResolvedValue(
new Response(JSON.stringify({ ok: true }), {
headers: { 'Content-Type': 'application/json' },
status: 200
})
);
const result = await requestWithMeta('/api/blocks/1/items');
expect(result.etag).toBeNull();
});
it('requestWithMeta surfaces the ETag on a 204 (no body) response', async () => {
vi.spyOn(window, 'fetch').mockResolvedValue(new Response(null, { headers: { ETag: '"3"' }, status: 204 }));
const result = await requestWithMeta('/api/blocks/1', { method: 'PUT' });
expect(result.data).toBeUndefined();
expect(result.etag).toBe('"3"');
});
});
+25 -4
View File
@@ -24,10 +24,21 @@ export interface ApiRequestOptions extends Omit<RequestInit, 'body'> {
const mutatingMethods = new Set(['DELETE', 'PATCH', 'POST', 'PUT']);
export async function request<TResponse = unknown>(
export interface ResponseWithMeta<T> {
data: T;
/** Strong ETag of the resource's version, when the endpoint emits one (issue #253). */
etag: string | null;
}
/**
* Like {@link request} but also returns the response's ETag. Used by the optimistic-concurrency
* editors: read the ETag from the load GET, send it back as `If-Match` on the replace PUT, and
* replace it from the PUT response's ETag on every successful save (issue #253 / spa-conventions).
*/
export async function requestWithMeta<TResponse = unknown>(
path: string,
options: ApiRequestOptions = {}
): Promise<TResponse> {
): Promise<ResponseWithMeta<TResponse>> {
const method = (options.method ?? 'GET').toUpperCase();
const headers = normalizeHeaders({
Accept: 'application/json',
@@ -52,11 +63,21 @@ export async function request<TResponse = unknown>(
throw new ApiError(response.status, await readProblemDetails(response));
}
const etag = response.headers.get('ETag');
if (response.status === 204) {
return undefined as TResponse;
return { data: undefined as TResponse, etag };
}
return await readJsonResponse(response) as TResponse;
return { data: (await readJsonResponse(response)) as TResponse, etag };
}
export async function request<TResponse = unknown>(
path: string,
options: ApiRequestOptions = {}
): Promise<TResponse> {
const { data } = await requestWithMeta<TResponse>(path, options);
return data;
}
function normalizeHeaders(headers: Record<string, string>): Record<string, string> {
+37
View File
@@ -205,6 +205,43 @@ describe('BlocksScreen', () => {
expect(sent.items.map((i: { searchTitle: string }) => i.searchTitle)).toEqual(['A', 'B']);
});
it('shows a conflict dialog and reloads when the block changed elsewhere (412)', async () => {
window.history.pushState({}, '', '/app/blocks/4');
let putCount = 0;
const fetchMock = mockApi({
items: [blockItem({ id: 1, searchTitle: 'A', searchQuery: 'a' })],
onRequest: (url, method) => {
if (url === '/api/blocks/4' && method === 'PUT') {
putCount += 1;
if (putCount === 1) {
return new Response(
JSON.stringify({ status: 412, title: 'Precondition Failed', detail: 'stale' }),
{ headers: { 'Content-Type': 'application/json' }, status: 412 }
);
}
}
return null;
}
});
render(<BlocksScreen />);
await waitFor(() => expect(screen.getByDisplayValue('Morning')).toBeInTheDocument());
const itemsGetCount = () =>
fetchMock.mock.calls.filter(([u, init]) => u === '/api/blocks/4/items' && (init?.method ?? 'GET') === 'GET')
.length;
const before = itemsGetCount();
fireEvent.click(screen.getByRole('button', { name: /Save block/ }));
// A 412 opens the "changed elsewhere" dialog rather than showing a generic save error.
expect(await screen.findByText(/Reload to get the latest version/i)).toBeInTheDocument();
// Reloading re-fetches the block items.
fireEvent.click(screen.getByRole('button', { name: /^Reload$/ }));
await waitFor(() => expect(itemsGetCount()).toBeGreaterThan(before));
});
it('round-trips legacy Artist and MultiCollection items through load and save', async () => {
window.history.pushState({}, '', '/app/blocks/4');
const fetchMock = mockApi({
+53 -10
View File
@@ -27,6 +27,7 @@ import {
Spinner
} from '../components';
import {
ApiError,
copyBlock,
createBlock,
createBlockGroup,
@@ -34,7 +35,7 @@ import {
deleteBlockGroup,
getBlock,
getBlockGroups,
getBlockItems,
getBlockItemsWithMeta,
getBlocks,
messageFromBlockError,
previewBlock,
@@ -853,15 +854,32 @@ function BlockEditor({ blockId }: { blockId: number }) {
const [previewItems, setPreviewItems] = useState<BlockPreviewItem[] | null>(null);
const [previewOpen, setPreviewOpen] = useState(false);
const [previewing, setPreviewing] = useState(false);
const [conflictOpen, setConflictOpen] = useState(false);
const [reloadKey, setReloadKey] = useState(0);
// Concurrency ETag (issue #253): captured from the items GET, sent as If-Match on save, and
// replaced from the PUT response on every successful save.
const etagRef = useRef<string | null>(null);
useEffect(() => {
let active = true;
Promise.all([getBlock(blockId), getBlockItems(blockId), getWatermarks(), getGraphicsElements()])
.then(([blockData, items, watermarkData, graphicsData]) => {
// Read items + ETag FIRST, then the root metadata. The ETag must be no newer than every piece of
// versioned data the draft is built from (issue #253): if the root were read first and a concurrent
// write landed before the items read, the draft would pair a stale root with a current ETag and the
// save would silently overwrite that write with no 412. Reading items first makes the ETag ≤ the
// root version, so any inconsistency fails safe (the save 412s → conflict dialog → reload).
void (async () => {
try {
const itemsMeta = await getBlockItemsWithMeta(blockId);
const [blockData, watermarkData, graphicsData] = await Promise.all([
getBlock(blockId),
getWatermarks(),
getGraphicsElements()
]);
if (!active) {
return;
}
etagRef.current = itemsMeta.etag;
setBlock(blockData);
setWatermarks(watermarkData);
setGraphicsElements(graphicsData);
@@ -870,19 +888,19 @@ function BlockEditor({ blockId }: { blockId: number }) {
hours: Math.floor(blockData.minutes / 60),
minutes: blockData.minutes % 60,
stopScheduling: blockData.stopScheduling,
items: items.map(itemFromResponse)
items: itemsMeta.data.map(itemFromResponse)
});
})
.catch((error: unknown) => {
} catch (error) {
if (active) {
setLoadError(messageFromBlockError(error, 'Unable to load block'));
}
});
}
})();
return () => {
active = false;
};
}, [blockId]);
}, [blockId, reloadKey]);
if (loadError) {
return (
@@ -972,15 +990,29 @@ function BlockEditor({ blockId }: { blockId: number }) {
setSaving(true);
setSaveError(null);
try {
await replaceBlock(blockId, toReplaceRequest(draft));
const { etag } = await replaceBlock(blockId, toReplaceRequest(draft), etagRef.current);
etagRef.current = etag;
navigateToPath(BASE_PATH);
} catch (error) {
setSaveError(messageFromBlockError(error, 'Unable to save block'));
if (error instanceof ApiError && error.status === 412) {
// Another edit landed since we loaded — force a reload rather than overwriting it (#253).
setConflictOpen(true);
} else {
setSaveError(messageFromBlockError(error, 'Unable to save block'));
}
} finally {
setSaving(false);
}
};
const reloadAfterConflict = () => {
setConflictOpen(false);
setSaveError(null);
setDraft(null);
setBlock(null);
setReloadKey((key) => key + 1);
};
const runPreview = async () => {
if (validationError || previewing) {
return;
@@ -1296,6 +1328,17 @@ function BlockEditor({ blockId }: { blockId: number }) {
<div className="ctv-collections-empty">No preview items were produced.</div>
)}
</Dialog>
<ConfirmDialog
cancelLabel="Keep editing"
confirmLabel="Reload"
message="This block was changed elsewhere since you opened it. Reload to get the latest version — your unsaved changes will be discarded."
onCancel={() => setConflictOpen(false)}
onConfirm={reloadAfterConflict}
open={conflictOpen}
title="Block changed elsewhere"
tone="danger"
/>
</div>
);
}