diff --git a/.husky/commit-msg b/.husky/commit-msg new file mode 100755 index 000000000..189629eac --- /dev/null +++ b/.husky/commit-msg @@ -0,0 +1,10 @@ +# Enforce the CLAUDE.md protocol: every commit message must carry a Co-Authored-By +# trailer. Merge commits are exempt (their MERGE_MSG has no trailer and shouldn't be +# rewritten). +if git rev-parse -q --verify MERGE_HEAD >/dev/null 2>&1; then + exit 0 +fi +grep -q '^Co-Authored-By:' "$1" || { + echo 'husky - commit message missing Co-Authored-By trailer' + exit 1 +} diff --git a/.husky/pre-commit b/.husky/pre-commit new file mode 100755 index 000000000..c55b0f41b --- /dev/null +++ b/.husky/pre-commit @@ -0,0 +1,15 @@ +cd web && npx lint-staged || exit 1 +cd .. + +# dotnet format on staged .cs files (repo root). Scoped to the staged files so we +# don't pay the full-tree cost; skip entirely when no .cs is staged (avoids the +# ~20-40s sln load for web-only commits). +cs_files=$(git diff --cached --name-only --diff-filter=ACM -- '*.cs') +if [ -n "$cs_files" ]; then + echo "husky - dotnet format (verify) on staged .cs files" + # shellcheck disable=SC2086 + dotnet format ErsatzTV.sln --verify-no-changes --include $cs_files || { + echo "husky - dotnet format found issues in staged .cs files; run 'dotnet format ErsatzTV.sln --include ' to fix" + exit 1 + } +fi diff --git a/.husky/pre-push b/.husky/pre-push new file mode 100755 index 000000000..cbc7cc985 --- /dev/null +++ b/.husky/pre-push @@ -0,0 +1,11 @@ +# Git exports GIT_DIR/GIT_WORK_TREE/GIT_INDEX_FILE while running hooks. In a worktree +# (or any subdir), an explicit GIT_DIR makes nested `git` commands mislocate the working +# tree — notably `check:api`'s `git diff --exit-code` (run from web/) silently reports "no +# diff" and lets drift through. Unset them so nested git rediscovers the repo normally. +unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE + +# CI-parity checks: catch "green locally, red in CI" before the push leaves the machine. +# check:api guards the generated OpenAPI types (v1.json / v1.d.ts drift); the full +# lint/typecheck/build catch a staged change that breaks an UNstaged file (lint-staged +# only sees staged files). +cd web && npm run check:api && npm run lint && npm run typecheck && npm run build diff --git a/ErsatzTV.Application/ConcurrencyExtensions.cs b/ErsatzTV.Application/ConcurrencyExtensions.cs index f77965b5c..cd64f2d03 100644 --- a/ErsatzTV.Application/ConcurrencyExtensions.cs +++ b/ErsatzTV.Application/ConcurrencyExtensions.cs @@ -1,11 +1,67 @@ using ErsatzTV.Core; +using ErsatzTV.Core.Domain; using ErsatzTV.Core.Errors; using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.ChangeTracking; namespace ErsatzTV.Application; public static class ConcurrencyExtensions { + /// + /// Persist changes that touch a versioned root but do not participate in the If-Match + /// contract (e.g. a playout's settings/schedule-file/on-demand-checkpoint writer, a collection's + /// name edit). Because the root's Version is an IsConcurrencyToken, EF guards every + /// UPDATE of that row with WHERE Version=@orig, so a concurrent bump from a replace-all + /// editor would otherwise surface as an unhandled → + /// 500 (issue #253 / #269). Phase-1 semantics for a missing If-Match is force-write, + /// so on a concurrency failure we adopt the stored token as both original (the retry's WHERE then + /// matches) and current (so we don't revert the concurrent bump) and retry — a client-wins merge + /// scoped to the token; our own modified scalars still win. Bounded to avoid a livelock; if the row + /// was deleted out from under us, that's a genuine conflict and rethrows. + /// + public static async Task SaveChangesForcingVersion( + this DbContext dbContext, + CancellationToken cancellationToken) + { + for (var attempt = 0; ; attempt++) + { + try + { + return await dbContext.SaveChangesAsync(cancellationToken); + } + catch (DbUpdateConcurrencyException ex) when (attempt < 5) + { + var resolvedAny = false; + foreach (EntityEntry entry in ex.Entries) + { + if (entry.Entity is not IVersionedAggregate) + { + continue; + } + + PropertyValues databaseValues = await entry.GetDatabaseValuesAsync(cancellationToken); + if (databaseValues is null) + { + // The row was deleted out from under us — a genuine conflict, not a token race. + throw; + } + + PropertyEntry version = entry.Property(nameof(IVersionedAggregate.Version)); + object currentVersion = databaseValues[nameof(IVersionedAggregate.Version)]!; + version.OriginalValue = currentVersion; + version.CurrentValue = currentVersion; + resolvedAny = true; + } + + if (!resolvedAny) + { + throw; + } + } + } + } + /// /// Persist pending changes, mapping the EF optimistic-concurrency failure to /// (→ 412). When a versioned root carries an diff --git a/ErsatzTV.Application/MediaCollections/Commands/AddEpisodeToPlaylistHandler.cs b/ErsatzTV.Application/MediaCollections/Commands/AddEpisodeToPlaylistHandler.cs index 43c0cac87..05cad00de 100644 --- a/ErsatzTV.Application/MediaCollections/Commands/AddEpisodeToPlaylistHandler.cs +++ b/ErsatzTV.Application/MediaCollections/Commands/AddEpisodeToPlaylistHandler.cs @@ -32,6 +32,10 @@ public class AddEpisodeToPlaylistHandler(IDbContextFactory dbContextF }; parameters.Playlist.Items.Add(playlistItem); + + // Mutates the playlist's editor-visible item list, so bump the concurrency token (issue #253). + parameters.Playlist.Version++; + await dbContext.SaveChangesAsync(); return Unit.Default; } diff --git a/ErsatzTV.Application/MediaCollections/Commands/AddItemsToPlaylistHandler.cs b/ErsatzTV.Application/MediaCollections/Commands/AddItemsToPlaylistHandler.cs index fd8586c21..f6ab29b8b 100644 --- a/ErsatzTV.Application/MediaCollections/Commands/AddItemsToPlaylistHandler.cs +++ b/ErsatzTV.Application/MediaCollections/Commands/AddItemsToPlaylistHandler.cs @@ -69,6 +69,9 @@ public class AddItemsToPlaylistHandler : IRequestHandler dbContextFac }; parameters.Playlist.Items.Add(playlistItem); + + // Mutates the playlist's editor-visible item list, so bump the concurrency token (issue #253). + parameters.Playlist.Version++; + await dbContext.SaveChangesAsync(); return Unit.Default; } diff --git a/ErsatzTV.Application/MediaCollections/Commands/AddSeasonToPlaylistHandler.cs b/ErsatzTV.Application/MediaCollections/Commands/AddSeasonToPlaylistHandler.cs index 40519bf6c..e9873bc46 100644 --- a/ErsatzTV.Application/MediaCollections/Commands/AddSeasonToPlaylistHandler.cs +++ b/ErsatzTV.Application/MediaCollections/Commands/AddSeasonToPlaylistHandler.cs @@ -32,6 +32,10 @@ public class AddSeasonToPlaylistHandler(IDbContextFactory dbContextFa }; parameters.Playlist.Items.Add(playlistItem); + + // Mutates the playlist's editor-visible item list, so bump the concurrency token (issue #253). + parameters.Playlist.Version++; + await dbContext.SaveChangesAsync(); return Unit.Default; } diff --git a/ErsatzTV.Application/MediaCollections/Commands/AddShowToPlaylistHandler.cs b/ErsatzTV.Application/MediaCollections/Commands/AddShowToPlaylistHandler.cs index 2795fd711..3159988ff 100644 --- a/ErsatzTV.Application/MediaCollections/Commands/AddShowToPlaylistHandler.cs +++ b/ErsatzTV.Application/MediaCollections/Commands/AddShowToPlaylistHandler.cs @@ -32,6 +32,10 @@ public class AddShowToPlaylistHandler(IDbContextFactory dbContextFact }; parameters.Playlist.Items.Add(playlistItem); + + // Mutates the playlist's editor-visible item list, so bump the concurrency token (issue #253). + parameters.Playlist.Version++; + await dbContext.SaveChangesAsync(); return Unit.Default; } diff --git a/ErsatzTV.Application/MediaCollections/Commands/ReplacePlaylistItems.cs b/ErsatzTV.Application/MediaCollections/Commands/ReplacePlaylistItems.cs index 74efb5729..caffd1928 100644 --- a/ErsatzTV.Application/MediaCollections/Commands/ReplacePlaylistItems.cs +++ b/ErsatzTV.Application/MediaCollections/Commands/ReplacePlaylistItems.cs @@ -2,5 +2,9 @@ using ErsatzTV.Core; namespace ErsatzTV.Application.MediaCollections; -public record ReplacePlaylistItems(int PlaylistId, string Name, List Items) +public record ReplacePlaylistItems( + int PlaylistId, + string Name, + List Items, + Option ExpectedVersion = default) : IRequest>>; diff --git a/ErsatzTV.Application/MediaCollections/Commands/ReplacePlaylistItemsHandler.cs b/ErsatzTV.Application/MediaCollections/Commands/ReplacePlaylistItemsHandler.cs index b55436918..85d26a50b 100644 --- a/ErsatzTV.Application/MediaCollections/Commands/ReplacePlaylistItemsHandler.cs +++ b/ErsatzTV.Application/MediaCollections/Commands/ReplacePlaylistItemsHandler.cs @@ -15,10 +15,21 @@ public class ReplacePlaylistItemsHandler(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). + // LanguageExtensions.ToEither joins the Seq to a single BaseError (the native + // Validation.ToEither() would keep Seq and is called explicitly here to avoid that shadow). + Either validated = LanguageExtensions.ToEither(validation) + .Bind(playlist => playlist.CheckVersion(request.ExpectedVersion)); + + return await validated.Match( + Right: playlist => Persist(dbContext, request, playlist, cancellationToken), + Left: error => Task.FromResult>>(error)); } - private static async Task> Persist( + private static async Task>> Persist( TvContext dbContext, ReplacePlaylistItems request, Playlist playlist, @@ -30,9 +41,15 @@ public class ReplacePlaylistItemsHandler(IDbContextFactory dbContextF dbContext.RemoveRange(playlist.Items); playlist.Items = request.Items.Map(i => BuildItem(playlist, 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). + playlist.Version++; - return playlist.Items.Map(Mapper.ProjectToViewModel).ToList(); + // 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 saved.Map(_ => playlist.Items.Map(Mapper.ProjectToViewModel).ToList()); } private static PlaylistItem BuildItem(Playlist playlist, int index, ReplacePlaylistItem item) => diff --git a/ErsatzTV.Application/MediaCollections/Commands/UpdateCollectionCustomOrder.cs b/ErsatzTV.Application/MediaCollections/Commands/UpdateCollectionCustomOrder.cs index 0e7c44b6b..6ae9d26c0 100644 --- a/ErsatzTV.Application/MediaCollections/Commands/UpdateCollectionCustomOrder.cs +++ b/ErsatzTV.Application/MediaCollections/Commands/UpdateCollectionCustomOrder.cs @@ -4,6 +4,7 @@ namespace ErsatzTV.Application.MediaCollections; public record UpdateCollectionCustomOrder( int CollectionId, - List MediaItemCustomOrders) : IRequest>; + List MediaItemCustomOrders, + Option ExpectedVersion = default) : IRequest>; public record MediaItemCustomOrder(int MediaItemId, int CustomIndex); diff --git a/ErsatzTV.Application/MediaCollections/Commands/UpdateCollectionCustomOrderHandler.cs b/ErsatzTV.Application/MediaCollections/Commands/UpdateCollectionCustomOrderHandler.cs index c8333f71b..50c20b11b 100644 --- a/ErsatzTV.Application/MediaCollections/Commands/UpdateCollectionCustomOrderHandler.cs +++ b/ErsatzTV.Application/MediaCollections/Commands/UpdateCollectionCustomOrderHandler.cs @@ -32,13 +32,22 @@ public class UpdateCollectionCustomOrderHandler : IRequestHandler validation = await Validate(dbContext, request, cancellationToken); - return await validation.Apply(c => ApplyUpdateRequest(dbContext, c, request)); + + // Optimistic-concurrency check as a standalone Either AFTER validation, never via Apply (which + // Join()s the error Seq and would flatten PreconditionFailedError to a 422) — issue #253 §7a. + Either validated = LanguageExtensions.ToEither(validation) + .Bind(c => c.CheckVersion(request.ExpectedVersion)); + + return await validated.Match( + Right: c => ApplyUpdateRequest(dbContext, c, request, cancellationToken), + Left: error => Task.FromResult>(error)); } - private async Task ApplyUpdateRequest( + private async Task> ApplyUpdateRequest( TvContext dbContext, Collection c, - UpdateCollectionCustomOrder request) + UpdateCollectionCustomOrder request, + CancellationToken cancellationToken) { foreach (MediaItemCustomOrder updateItem in request.MediaItemCustomOrders) { @@ -51,14 +60,24 @@ public class UpdateCollectionCustomOrderHandler : IRequestHandler 0) + // Unconditional bump (issue #253 §7a / M1) then guarded save (→ 412 on a lost race). + c.Version++; + Either saveResult = await dbContext.SaveChangesWithConcurrencyGuard(cancellationToken); + if (saveResult.IsLeft) { - // refresh all playouts that use this collection - foreach (int playoutId in await _mediaCollectionRepository - .PlayoutIdsUsingCollection(request.CollectionId)) - { - await _channel.WriteAsync(new BuildPlayout(playoutId, PlayoutBuildMode.Refresh)); - } + return saveResult; + } + + // Refresh all playouts that use this collection. The old `SaveChangesAsync() > 0` gate is always + // true once the version bumps unconditionally (M2), so run the refresh on any successful save. + // Post-commit enqueue on CancellationToken.None so a late cancellation can't drop the rebuild + // after the commit landed (#254 / §7b). + foreach (int playoutId in await _mediaCollectionRepository + .PlayoutIdsUsingCollection(request.CollectionId)) + { + await _channel.WriteAsync( + new BuildPlayout(playoutId, PlayoutBuildMode.Refresh), + CancellationToken.None); } return Unit.Default; diff --git a/ErsatzTV.Application/MediaCollections/Commands/UpdateCollectionHandler.cs b/ErsatzTV.Application/MediaCollections/Commands/UpdateCollectionHandler.cs index 73ca500dd..5b0830eb6 100644 --- a/ErsatzTV.Application/MediaCollections/Commands/UpdateCollectionHandler.cs +++ b/ErsatzTV.Application/MediaCollections/Commands/UpdateCollectionHandler.cs @@ -59,7 +59,9 @@ public class UpdateCollectionHandler : IRequestHandler 0 && request.UseCustomPlaybackOrder.IsSome) + // Force-write past a concurrent Version bump from the custom-order editor (this name/flag writer + // doesn't participate in If-Match, so the active token must not 500 a benign race) — #253/#269. + if (await dbContext.SaveChangesForcingVersion(cancellationToken) > 0 && request.UseCustomPlaybackOrder.IsSome) { // post-commit side effect runs on CancellationToken.None so a late request cancellation // can't abort it after the commit landed (#254) diff --git a/ErsatzTV.Application/MediaCollections/Commands/UpdateMultiCollection.cs b/ErsatzTV.Application/MediaCollections/Commands/UpdateMultiCollection.cs index 9aca8d253..13e156667 100644 --- a/ErsatzTV.Application/MediaCollections/Commands/UpdateMultiCollection.cs +++ b/ErsatzTV.Application/MediaCollections/Commands/UpdateMultiCollection.cs @@ -12,4 +12,5 @@ public record UpdateMultiCollectionItem( public record UpdateMultiCollection( int MultiCollectionId, string Name, - List Items) : IRequest>; + List Items, + Option ExpectedVersion = default) : IRequest>; diff --git a/ErsatzTV.Application/MediaCollections/Commands/UpdateMultiCollectionHandler.cs b/ErsatzTV.Application/MediaCollections/Commands/UpdateMultiCollectionHandler.cs index f4af1f30a..877ad55a6 100644 --- a/ErsatzTV.Application/MediaCollections/Commands/UpdateMultiCollectionHandler.cs +++ b/ErsatzTV.Application/MediaCollections/Commands/UpdateMultiCollectionHandler.cs @@ -36,10 +36,18 @@ public class UpdateMultiCollectionHandler : IRequestHandler validation = await Validate(dbContext, request, cancellationToken); - return await validation.Apply(c => ApplyUpdateRequest(dbContext, c, request, cancellationToken)); + + // Optimistic-concurrency check as a standalone Either AFTER validation, never via Apply (which + // Join()s the error Seq and would flatten PreconditionFailedError to a 422) — issue #253 §7a. + Either validated = LanguageExtensions.ToEither(validation) + .Bind(c => c.CheckVersion(request.ExpectedVersion)); + + return await validated.Match( + Right: c => ApplyUpdateRequest(dbContext, c, request, cancellationToken), + Left: error => Task.FromResult>(error)); } - private async Task ApplyUpdateRequest( + private async Task> ApplyUpdateRequest( TvContext dbContext, MultiCollection c, UpdateMultiCollection request, @@ -47,8 +55,16 @@ public class UpdateMultiCollectionHandler : IRequestHandler nameSave = await dbContext.SaveChangesWithConcurrencyGuard(cancellationToken); + if (nameSave.IsLeft) + { + return nameSave; + } var toAdd = request.Items .Filter(i => i.CollectionId.HasValue) diff --git a/ErsatzTV.Application/MediaCollections/Commands/UpdateRerunCollection.cs b/ErsatzTV.Application/MediaCollections/Commands/UpdateRerunCollection.cs index 089593d00..11adece2d 100644 --- a/ErsatzTV.Application/MediaCollections/Commands/UpdateRerunCollection.cs +++ b/ErsatzTV.Application/MediaCollections/Commands/UpdateRerunCollection.cs @@ -13,5 +13,6 @@ public record UpdateRerunCollection( SmartCollectionViewModel SmartCollection, NamedMediaItemViewModel MediaItem, PlaybackOrder FirstRunPlaybackOrder, - PlaybackOrder RerunPlaybackOrder) + PlaybackOrder RerunPlaybackOrder, + Option ExpectedVersion = default) : IRequest>; diff --git a/ErsatzTV.Application/MediaCollections/Commands/UpdateRerunCollectionHandler.cs b/ErsatzTV.Application/MediaCollections/Commands/UpdateRerunCollectionHandler.cs index 3f2c3620f..7a9a68f99 100644 --- a/ErsatzTV.Application/MediaCollections/Commands/UpdateRerunCollectionHandler.cs +++ b/ErsatzTV.Application/MediaCollections/Commands/UpdateRerunCollectionHandler.cs @@ -22,10 +22,18 @@ public class UpdateRerunCollectionHandler( { await using TvContext dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); Validation validation = await Validate(dbContext, request, cancellationToken); - return await validation.Apply(c => ApplyUpdateRequest(dbContext, c, request, cancellationToken)); + + // Optimistic-concurrency check as a standalone Either AFTER validation, never via Apply (which + // Join()s the error Seq and would flatten PreconditionFailedError to a 422) — issue #253 §7a. + Either validated = LanguageExtensions.ToEither(validation) + .Bind(c => c.CheckVersion(request.ExpectedVersion)); + + return await validated.Match( + Right: c => ApplyUpdateRequest(dbContext, c, request, cancellationToken), + Left: error => Task.FromResult>(error)); } - private async Task ApplyUpdateRequest( + private async Task> ApplyUpdateRequest( TvContext dbContext, RerunCollection c, UpdateRerunCollection request, @@ -40,17 +48,23 @@ public class UpdateRerunCollectionHandler( c.FirstRunPlaybackOrder = request.FirstRunPlaybackOrder; c.RerunPlaybackOrder = request.RerunPlaybackOrder; - // rebuild playouts - if (await dbContext.SaveChangesAsync(cancellationToken) > 0) + // Unconditional bump (issue #253 §7a / M1) then guarded save, which maps a racing + // DbUpdateConcurrencyException to PreconditionFailedError (→ 412). + c.Version++; + Either saveResult = await dbContext.SaveChangesWithConcurrencyGuard(cancellationToken); + if (saveResult.IsLeft) { - // post-commit side effect runs on CancellationToken.None so a late request cancellation - // can't abort it after the commit landed (#254) - // refresh all playouts that use this rerun collection - foreach (int playoutId in await mediaCollectionRepository.PlayoutIdsUsingRerunCollection( - request.RerunCollectionId)) - { - await channel.WriteAsync(new BuildPlayout(playoutId, PlayoutBuildMode.Refresh), CancellationToken.None); - } + return saveResult; + } + + // Refresh all playouts that use this rerun collection. The old `SaveChangesAsync() > 0` gate is + // always true once the version bumps unconditionally (M2), so run the refresh on any successful + // save. Post-commit enqueue on CancellationToken.None so a late cancellation can't drop the + // rebuild after the commit landed (#254). + foreach (int playoutId in await mediaCollectionRepository.PlayoutIdsUsingRerunCollection( + request.RerunCollectionId)) + { + await channel.WriteAsync(new BuildPlayout(playoutId, PlayoutBuildMode.Refresh), CancellationToken.None); } return Unit.Default; diff --git a/ErsatzTV.Application/MediaCollections/Mapper.cs b/ErsatzTV.Application/MediaCollections/Mapper.cs index 05271e474..c724bcde4 100644 --- a/ErsatzTV.Application/MediaCollections/Mapper.cs +++ b/ErsatzTV.Application/MediaCollections/Mapper.cs @@ -12,14 +12,16 @@ internal static class Mapper collection.Id, collection.Name, collection.UseCustomPlaybackOrder, - MediaItemState.Normal); + MediaItemState.Normal, + collection.Version); internal static MultiCollectionViewModel ProjectToViewModel(MultiCollection multiCollection) => new( multiCollection.Id, multiCollection.Name, Optional(multiCollection.MultiCollectionItems).Flatten().Map(ProjectToViewModel).ToList(), - Optional(multiCollection.MultiCollectionSmartItems).Flatten().Map(ProjectToViewModel).ToList()); + Optional(multiCollection.MultiCollectionSmartItems).Flatten().Map(ProjectToViewModel).ToList(), + multiCollection.Version); internal static SmartCollectionViewModel ProjectToViewModel(SmartCollection collection) => new(collection.Id, collection.Name, collection.Query); @@ -49,7 +51,8 @@ internal static class Mapper _ => null }, collection.FirstRunPlaybackOrder, - collection.RerunPlaybackOrder); + collection.RerunPlaybackOrder, + collection.Version); internal static TraktListViewModel ProjectToViewModel(TraktList traktList) => new( @@ -89,7 +92,7 @@ internal static class Mapper new(playlistGroup.Id, playlistGroup.Name, playlistGroup.Playlists.Count, playlistGroup.IsSystem); internal static PlaylistViewModel ProjectToViewModel(Playlist playlist) => - new(playlist.Id, playlist.PlaylistGroupId, playlist.Name, playlist.IsSystem); + new(playlist.Id, playlist.PlaylistGroupId, playlist.Name, playlist.IsSystem, playlist.Version); internal static PlaylistItemViewModel ProjectToViewModel(PlaylistItem playlistItem) => new( diff --git a/ErsatzTV.Application/MediaCollections/MediaCollectionViewModel.cs b/ErsatzTV.Application/MediaCollections/MediaCollectionViewModel.cs index 023842a04..8bd43b956 100644 --- a/ErsatzTV.Application/MediaCollections/MediaCollectionViewModel.cs +++ b/ErsatzTV.Application/MediaCollections/MediaCollectionViewModel.cs @@ -8,7 +8,10 @@ public record MediaCollectionViewModel( int Id, string Name, bool UseCustomPlaybackOrder, - MediaItemState State) : MediaCardViewModel( + MediaItemState State, + // Optimistic-concurrency token (issue #253), header-only via ETag; 0 for the selection/placeholder + // constructions that don't read a real collection. Set from the entity by the Mapper projection. + int Version = 0) : MediaCardViewModel( Id, Name, string.Empty, diff --git a/ErsatzTV.Application/MediaCollections/MultiCollectionViewModel.cs b/ErsatzTV.Application/MediaCollections/MultiCollectionViewModel.cs index 507826fd7..e90ebc916 100644 --- a/ErsatzTV.Application/MediaCollections/MultiCollectionViewModel.cs +++ b/ErsatzTV.Application/MediaCollections/MultiCollectionViewModel.cs @@ -4,4 +4,6 @@ public record MultiCollectionViewModel( int Id, string Name, List Items, - List SmartItems); + List SmartItems, + // Optimistic-concurrency token (issue #253), header-only via ETag; 0 for selection placeholders. + int Version = 0); diff --git a/ErsatzTV.Application/MediaCollections/PlaylistViewModel.cs b/ErsatzTV.Application/MediaCollections/PlaylistViewModel.cs index 0b57c33ae..febb7e254 100644 --- a/ErsatzTV.Application/MediaCollections/PlaylistViewModel.cs +++ b/ErsatzTV.Application/MediaCollections/PlaylistViewModel.cs @@ -1,3 +1,3 @@ namespace ErsatzTV.Application.MediaCollections; -public record PlaylistViewModel(int Id, int PlaylistGroupId, string Name, bool IsSystem); +public record PlaylistViewModel(int Id, int PlaylistGroupId, string Name, bool IsSystem, int Version); diff --git a/ErsatzTV.Application/MediaCollections/RerunCollectionViewModel.cs b/ErsatzTV.Application/MediaCollections/RerunCollectionViewModel.cs index 3be4e1497..27d1415ff 100644 --- a/ErsatzTV.Application/MediaCollections/RerunCollectionViewModel.cs +++ b/ErsatzTV.Application/MediaCollections/RerunCollectionViewModel.cs @@ -12,4 +12,6 @@ public record RerunCollectionViewModel( SmartCollectionViewModel SmartCollection, NamedMediaItemViewModel MediaItem, PlaybackOrder FirstRunPlaybackOrder, - PlaybackOrder RerunPlaybackOrder); + PlaybackOrder RerunPlaybackOrder, + // Optimistic-concurrency token (issue #253), header-only via ETag. + int Version = 0); diff --git a/ErsatzTV.Application/Playouts/Commands/ReplacePlayoutAlternateScheduleItems.cs b/ErsatzTV.Application/Playouts/Commands/ReplacePlayoutAlternateScheduleItems.cs index dd5b7fc78..7c398fceb 100644 --- a/ErsatzTV.Application/Playouts/Commands/ReplacePlayoutAlternateScheduleItems.cs +++ b/ErsatzTV.Application/Playouts/Commands/ReplacePlayoutAlternateScheduleItems.cs @@ -2,5 +2,8 @@ using ErsatzTV.Core; namespace ErsatzTV.Application.Playouts; -public record ReplacePlayoutAlternateScheduleItems(int PlayoutId, List Items) +public record ReplacePlayoutAlternateScheduleItems( + int PlayoutId, + List Items, + Option ExpectedVersion = default) : IRequest>; diff --git a/ErsatzTV.Application/Playouts/Commands/ReplacePlayoutAlternateScheduleItemsHandler.cs b/ErsatzTV.Application/Playouts/Commands/ReplacePlayoutAlternateScheduleItemsHandler.cs index 434a0e439..8cc641b74 100644 --- a/ErsatzTV.Application/Playouts/Commands/ReplacePlayoutAlternateScheduleItemsHandler.cs +++ b/ErsatzTV.Application/Playouts/Commands/ReplacePlayoutAlternateScheduleItemsHandler.cs @@ -40,6 +40,18 @@ public class ReplacePlayoutAlternateScheduleItemsHandler( foreach (Playout playout in maybePlayout) { + // Optimistic-concurrency pre-check (issue #253 §7a): reject a stale If-Match with a + // PreconditionFailedError (→ 412) before any mutation. Introduced as a standalone Either, + // and returned directly (a value, not a throw) so it escapes the catch(Exception) below + // rather than being reshaped into a bare 422 (§9/H1). + Either versionCheck = playout.CheckVersion(request.ExpectedVersion); + if (versionCheck.IsLeft) + { + return versionCheck.Match>( + Left: error => error, + Right: _ => Unit.Default); + } + var existingScheduleMap = new Dictionary(); var daysToCheck = new List(); @@ -127,7 +139,20 @@ public class ReplacePlayoutAlternateScheduleItemsHandler( playout.ProgramScheduleId = highest.ProgramScheduleId; } - await dbContext.SaveChangesAsync(cancellationToken); + // Unconditional bump (issue #253 §7a / M1): EF emits the root UPDATE only when a scalar + // actually differs, so a no-op PUT-back would otherwise neither fire the concurrency token + // nor rotate other clients' ETags. + playout.Version++; + + // Guarded save maps a racing DbUpdateConcurrencyException to PreconditionFailedError (→ 412) + // as a return value, so a lost race short-circuits here before the post-commit refresh block + // and escapes the catch(Exception) below as a 412, not a 422 (§9/H1). + Either saveResult = + await dbContext.SaveChangesWithConcurrencyGuard(cancellationToken); + if (saveResult.IsLeft) + { + return saveResult; + } if (hasDefaultScheduleChange) { diff --git a/ErsatzTV.Application/Playouts/Commands/UpdateExternalJsonPlayoutHandler.cs b/ErsatzTV.Application/Playouts/Commands/UpdateExternalJsonPlayoutHandler.cs index c94770f57..2ea547c66 100644 --- a/ErsatzTV.Application/Playouts/Commands/UpdateExternalJsonPlayoutHandler.cs +++ b/ErsatzTV.Application/Playouts/Commands/UpdateExternalJsonPlayoutHandler.cs @@ -40,7 +40,9 @@ public class { playout.ScheduleFile = request.ScheduleFile; - if (await dbContext.SaveChangesAsync(cancellationToken) > 0) + // Force-write past a concurrent Version bump from a replace-all editor (schedule-file writer + // doesn't participate in If-Match, so an active token must not 500 a benign race) — #253/#269. + if (await dbContext.SaveChangesForcingVersion(cancellationToken) > 0) { // post-commit side effect runs on CancellationToken.None so a late request cancellation // can't abort it after the commit landed (#254) @@ -58,7 +60,8 @@ public class playout.DailyRebuildTime, playout.BuildStatus, playout.DecoId, - playout.Deco?.Name); + playout.Deco?.Name, + playout.Version); } private static Task> Validate( diff --git a/ErsatzTV.Application/Playouts/Commands/UpdateOnDemandCheckpointHandler.cs b/ErsatzTV.Application/Playouts/Commands/UpdateOnDemandCheckpointHandler.cs index 19c15367d..e5262971d 100644 --- a/ErsatzTV.Application/Playouts/Commands/UpdateOnDemandCheckpointHandler.cs +++ b/ErsatzTV.Application/Playouts/Commands/UpdateOnDemandCheckpointHandler.cs @@ -61,7 +61,10 @@ public class UpdateOnDemandCheckpointHandler( playout.Channel.Name, playout.OnDemandCheckpoint); - await dbContext.SaveChangesAsync(cancellationToken); + // Force-write past a concurrent Version bump from a replace-all editor: the on-demand + // checkpoint writer touches the Playout root but doesn't participate in If-Match, so the + // active concurrency token must not turn a benign race into a 500 (#253/#269). + await dbContext.SaveChangesForcingVersion(cancellationToken); } } } diff --git a/ErsatzTV.Application/Playouts/Commands/UpdatePlayoutHandler.cs b/ErsatzTV.Application/Playouts/Commands/UpdatePlayoutHandler.cs index 1ca83a979..3ede2fdb6 100644 --- a/ErsatzTV.Application/Playouts/Commands/UpdatePlayoutHandler.cs +++ b/ErsatzTV.Application/Playouts/Commands/UpdatePlayoutHandler.cs @@ -35,7 +35,9 @@ public class UpdatePlayoutHandler : IRequestHandler> Validate( diff --git a/ErsatzTV.Application/Playouts/Commands/UpdateScriptedPlayoutHandler.cs b/ErsatzTV.Application/Playouts/Commands/UpdateScriptedPlayoutHandler.cs index 9ef7c1c58..76d755e1c 100644 --- a/ErsatzTV.Application/Playouts/Commands/UpdateScriptedPlayoutHandler.cs +++ b/ErsatzTV.Application/Playouts/Commands/UpdateScriptedPlayoutHandler.cs @@ -35,7 +35,9 @@ public class { playout.ScheduleFile = request.ScheduleFile; - if (await dbContext.SaveChangesAsync(cancellationToken) > 0) + // Force-write past a concurrent Version bump from a replace-all editor (schedule-file writer + // doesn't participate in If-Match, so an active token must not 500 a benign race) — #253/#269. + if (await dbContext.SaveChangesForcingVersion(cancellationToken) > 0) { // post-commit side effect runs on CancellationToken.None so a late request cancellation // can't abort it after the commit landed (#254) @@ -53,7 +55,8 @@ public class playout.DailyRebuildTime, playout.BuildStatus, playout.DecoId, - playout.Deco?.Name); + playout.Deco?.Name, + playout.Version); } private async Task> Validate( diff --git a/ErsatzTV.Application/Playouts/Commands/UpdateSequentialPlayoutHandler.cs b/ErsatzTV.Application/Playouts/Commands/UpdateSequentialPlayoutHandler.cs index e81cb6ab9..55bd00319 100644 --- a/ErsatzTV.Application/Playouts/Commands/UpdateSequentialPlayoutHandler.cs +++ b/ErsatzTV.Application/Playouts/Commands/UpdateSequentialPlayoutHandler.cs @@ -40,7 +40,9 @@ public class { playout.ScheduleFile = request.ScheduleFile; - if (await dbContext.SaveChangesAsync(cancellationToken) > 0) + // Force-write past a concurrent Version bump from a replace-all editor (schedule-file writer + // doesn't participate in If-Match, so an active token must not 500 a benign race) — #253/#269. + if (await dbContext.SaveChangesForcingVersion(cancellationToken) > 0) { // post-commit side effect runs on CancellationToken.None so a late request cancellation // can't abort it after the commit landed (#254) @@ -58,7 +60,8 @@ public class playout.DailyRebuildTime, playout.BuildStatus, playout.DecoId, - playout.Deco?.Name); + playout.Deco?.Name, + playout.Version); } private static Task> Validate( diff --git a/ErsatzTV.Application/Playouts/Mapper.cs b/ErsatzTV.Application/Playouts/Mapper.cs index da82809b5..9f05feaae 100644 --- a/ErsatzTV.Application/Playouts/Mapper.cs +++ b/ErsatzTV.Application/Playouts/Mapper.cs @@ -19,7 +19,8 @@ internal static class Mapper playout.DecoId, // the paged-playouts query does not eager-load Deco (the list response does not surface // the default deco); GetPlayoutById includes it for the detail response - playout.Deco?.Name); + playout.Deco?.Name, + playout.Version); internal static PlayoutItemViewModel ProjectToViewModel(PlayoutItem playoutItem) => new( diff --git a/ErsatzTV.Application/Playouts/PlayoutNameViewModel.cs b/ErsatzTV.Application/Playouts/PlayoutNameViewModel.cs index 6ee27fbf0..82a1d1bcd 100644 --- a/ErsatzTV.Application/Playouts/PlayoutNameViewModel.cs +++ b/ErsatzTV.Application/Playouts/PlayoutNameViewModel.cs @@ -13,7 +13,8 @@ public record PlayoutNameViewModel( TimeSpan? DbDailyRebuildTime, PlayoutBuildStatus BuildStatus, int? DecoId, - string DecoName) + string DecoName, + int Version) { public Option DailyRebuildTime => Optional(DbDailyRebuildTime); diff --git a/ErsatzTV.Application/Playouts/Queries/GetPlayoutByIdHandler.cs b/ErsatzTV.Application/Playouts/Queries/GetPlayoutByIdHandler.cs index 8583f91ac..00439caaf 100644 --- a/ErsatzTV.Application/Playouts/Queries/GetPlayoutByIdHandler.cs +++ b/ErsatzTV.Application/Playouts/Queries/GetPlayoutByIdHandler.cs @@ -30,6 +30,7 @@ public class GetPlayoutByIdHandler(IDbContextFactory dbContextFactory p.DailyRebuildTime, p.BuildStatus, p.DecoId, - p.DecoId == null ? null : p.Deco.Name)); + p.DecoId == null ? null : p.Deco.Name, + p.Version)); } } diff --git a/ErsatzTV.Application/ProgramSchedules/Commands/AddProgramScheduleItemHandler.cs b/ErsatzTV.Application/ProgramSchedules/Commands/AddProgramScheduleItemHandler.cs index b8656406a..93f2862b6 100644 --- a/ErsatzTV.Application/ProgramSchedules/Commands/AddProgramScheduleItemHandler.cs +++ b/ErsatzTV.Application/ProgramSchedules/Commands/AddProgramScheduleItemHandler.cs @@ -52,6 +52,9 @@ public class AddProgramScheduleItemHandler : ProgramScheduleItemCommandBase, ProgramScheduleItem item = BuildItem(programSchedule, nextIndex, request); programSchedule.Items.Add(item); + // bump the optimistic-concurrency token so this config edit rotates other clients' ETags (#253) + programSchedule.Version++; + await dbContext.SaveChangesAsync(cancellationToken); // refresh any playouts that use this schedule diff --git a/ErsatzTV.Application/ProgramSchedules/Commands/DeleteProgramScheduleItemHandler.cs b/ErsatzTV.Application/ProgramSchedules/Commands/DeleteProgramScheduleItemHandler.cs index 017f64ac5..5a610c97f 100644 --- a/ErsatzTV.Application/ProgramSchedules/Commands/DeleteProgramScheduleItemHandler.cs +++ b/ErsatzTV.Application/ProgramSchedules/Commands/DeleteProgramScheduleItemHandler.cs @@ -34,6 +34,10 @@ public class DeleteProgramScheduleItemHandler( List playouts = item.ProgramSchedule.Playouts; dbContext.ProgramScheduleItems.Remove(item); + + // bump the optimistic-concurrency token so this config edit rotates other clients' ETags (#253) + item.ProgramSchedule.Version++; + await dbContext.SaveChangesAsync(cancellationToken); // post-commit side effect runs on CancellationToken.None so a late request cancellation diff --git a/ErsatzTV.Application/ProgramSchedules/Commands/ReplaceProgramScheduleItems.cs b/ErsatzTV.Application/ProgramSchedules/Commands/ReplaceProgramScheduleItems.cs index 0c3c0b8ec..9a9d4568c 100644 --- a/ErsatzTV.Application/ProgramSchedules/Commands/ReplaceProgramScheduleItems.cs +++ b/ErsatzTV.Application/ProgramSchedules/Commands/ReplaceProgramScheduleItems.cs @@ -44,5 +44,8 @@ public record ReplaceProgramScheduleItem( string PreferredSubtitleLanguageCode, ChannelSubtitleMode? SubtitleMode) : IProgramScheduleItemRequest; -public record ReplaceProgramScheduleItems(int ProgramScheduleId, List Items) : IRequest< +public record ReplaceProgramScheduleItems( + int ProgramScheduleId, + List Items, + Option ExpectedVersion = default) : IRequest< Either>>; diff --git a/ErsatzTV.Application/ProgramSchedules/Commands/ReplaceProgramScheduleItemsHandler.cs b/ErsatzTV.Application/ProgramSchedules/Commands/ReplaceProgramScheduleItemsHandler.cs index f68ac5e73..43a626177 100644 --- a/ErsatzTV.Application/ProgramSchedules/Commands/ReplaceProgramScheduleItemsHandler.cs +++ b/ErsatzTV.Application/ProgramSchedules/Commands/ReplaceProgramScheduleItemsHandler.cs @@ -26,13 +26,23 @@ public class ReplaceProgramScheduleItemsHandler( Some: async programSchedule => { Validation validation = await Validate(dbContext, request, programSchedule); - return await validation.Apply(ps => PersistItems(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(ps => ps.CheckVersion(request.ExpectedVersion)); + + return await validated.Match( + Right: ps => PersistItems(dbContext, request, ps, cancellationToken), + Left: error => + Task.FromResult>>(error)); }, None: () => Task.FromResult>>( new NotFoundError("[ProgramScheduleId] does not exist."))); } - private async Task> PersistItems( + private async Task>> PersistItems( TvContext dbContext, ReplaceProgramScheduleItems request, ProgramSchedule programSchedule, @@ -92,7 +102,20 @@ public class ReplaceProgramScheduleItemsHandler( programSchedule.Items.Add(BuildItem(programSchedule, i, orderedItems[i])); } - await dbContext.SaveChangesAsync(cancellationToken); + // Unconditional bump: this handler frequently saves with only CHILD changes and no root-scalar + // change, so without an explicit bump EF would emit no root UPDATE and the concurrency token + // would never fire (nor rotate other clients' ETags). Bumping guarantees both on every save, + // including a no-op same-items PUT-back (issue #253 / api-conventions §7a). + programSchedule.Version++; + + // 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. On failure, propagate the error WITHOUT + // running the post-save reload/enqueue below. + Either saved = await dbContext.SaveChangesWithConcurrencyGuard(cancellationToken); + if (saved.IsLeft) + { + return saved.Map(_ => (IEnumerable)[]); + } // refresh any playouts that use this schedule // post-commit side effect runs on CancellationToken.None so a late request cancellation diff --git a/ErsatzTV.Application/ProgramSchedules/Commands/UpdateProgramScheduleHandler.cs b/ErsatzTV.Application/ProgramSchedules/Commands/UpdateProgramScheduleHandler.cs index 9198db1a3..6500e9b5a 100644 --- a/ErsatzTV.Application/ProgramSchedules/Commands/UpdateProgramScheduleHandler.cs +++ b/ErsatzTV.Application/ProgramSchedules/Commands/UpdateProgramScheduleHandler.cs @@ -54,6 +54,9 @@ public class UpdateProgramScheduleHandler( programSchedule.RandomStartPoint = request.RandomStartPoint; programSchedule.FixedStartTimeBehavior = request.FixedStartTimeBehavior; + // bump the optimistic-concurrency token so this config edit rotates other clients' ETags (#253) + programSchedule.Version++; + await dbContext.SaveChangesAsync(); if (needToRefreshPlayout) diff --git a/ErsatzTV.Application/ProgramSchedules/Mapper.cs b/ErsatzTV.Application/ProgramSchedules/Mapper.cs index 43460b45b..21f1821d0 100644 --- a/ErsatzTV.Application/ProgramSchedules/Mapper.cs +++ b/ErsatzTV.Application/ProgramSchedules/Mapper.cs @@ -12,7 +12,8 @@ internal static class Mapper programSchedule.TreatCollectionsAsShows, programSchedule.ShuffleScheduleItems, programSchedule.RandomStartPoint, - programSchedule.FixedStartTimeBehavior); + programSchedule.FixedStartTimeBehavior, + programSchedule.Version); internal static ProgramScheduleItemViewModel ProjectToViewModel(ProgramScheduleItem programScheduleItem) => programScheduleItem switch diff --git a/ErsatzTV.Application/ProgramSchedules/ProgramScheduleViewModel.cs b/ErsatzTV.Application/ProgramSchedules/ProgramScheduleViewModel.cs index c8750ad4e..1eee27acf 100644 --- a/ErsatzTV.Application/ProgramSchedules/ProgramScheduleViewModel.cs +++ b/ErsatzTV.Application/ProgramSchedules/ProgramScheduleViewModel.cs @@ -9,4 +9,5 @@ public record ProgramScheduleViewModel( bool TreatCollectionsAsShows, bool ShuffleScheduleItems, bool RandomStartPoint, - FixedStartTimeBehavior FixedStartTimeBehavior); + FixedStartTimeBehavior FixedStartTimeBehavior, + int Version); diff --git a/ErsatzTV.Application/ProgramSchedules/Queries/GetAllProgramSchedulesHandler.cs b/ErsatzTV.Application/ProgramSchedules/Queries/GetAllProgramSchedulesHandler.cs index c13f160eb..1abe7626f 100644 --- a/ErsatzTV.Application/ProgramSchedules/Queries/GetAllProgramSchedulesHandler.cs +++ b/ErsatzTV.Application/ProgramSchedules/Queries/GetAllProgramSchedulesHandler.cs @@ -19,7 +19,8 @@ public class GetAllProgramSchedulesHandler(IDbContextFactory dbContex ps.TreatCollectionsAsShows, ps.ShuffleScheduleItems, ps.RandomStartPoint, - ps.FixedStartTimeBehavior)) + ps.FixedStartTimeBehavior, + ps.Version)) .ToListAsync(cancellationToken); } } 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/ReplacePlayoutTemplateItems.cs b/ErsatzTV.Application/Scheduling/Commands/ReplacePlayoutTemplateItems.cs index 221d19cbf..00888c406 100644 --- a/ErsatzTV.Application/Scheduling/Commands/ReplacePlayoutTemplateItems.cs +++ b/ErsatzTV.Application/Scheduling/Commands/ReplacePlayoutTemplateItems.cs @@ -2,5 +2,8 @@ using ErsatzTV.Core; namespace ErsatzTV.Application.Scheduling; -public record ReplacePlayoutTemplateItems(int PlayoutId, List Items) +public record ReplacePlayoutTemplateItems( + int PlayoutId, + List Items, + Option ExpectedVersion = default) : IRequest>; diff --git a/ErsatzTV.Application/Scheduling/Commands/ReplacePlayoutTemplateItemsHandler.cs b/ErsatzTV.Application/Scheduling/Commands/ReplacePlayoutTemplateItemsHandler.cs index a9c33ffdd..11975e491 100644 --- a/ErsatzTV.Application/Scheduling/Commands/ReplacePlayoutTemplateItemsHandler.cs +++ b/ErsatzTV.Application/Scheduling/Commands/ReplacePlayoutTemplateItemsHandler.cs @@ -29,6 +29,17 @@ public class ReplacePlayoutTemplateItemsHandler( foreach (Playout playout in maybePlayout) { + // Optimistic-concurrency pre-check (issue #253 §7a): reject a stale If-Match with a + // PreconditionFailedError (→ 412) before any mutation. Returned directly so it escapes the + // catch(Exception) below as a 412 rather than being reshaped into a bare 422 (§9/H1). + Either versionCheck = playout.CheckVersion(request.ExpectedVersion); + if (versionCheck.IsLeft) + { + return versionCheck.Match>( + Left: error => error, + Right: _ => Option.None); + } + PlayoutTemplate[] existing = playout.Templates.ToArray(); List incoming = request.Items; @@ -88,7 +99,20 @@ public class ReplacePlayoutTemplateItemsHandler( } } - await dbContext.SaveChangesAsync(cancellationToken); + // Unconditional bump (issue #253 §7a / M1): #8 writes no root scalar at all (only the + // Templates child collection), so without this the root UPDATE never fires and the token + // never rotates. Bump then save through the guard, which maps a racing + // DbUpdateConcurrencyException to PreconditionFailedError (→ 412) as a return value. + playout.Version++; + + Either saveResult = + await dbContext.SaveChangesWithConcurrencyGuard(cancellationToken); + if (saveResult.IsLeft) + { + return saveResult.Match>( + Left: error => error, + Right: _ => Option.None); + } } return Option.None; 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/Commands/UpdateDefaultDecoHandler.cs b/ErsatzTV.Application/Scheduling/Commands/UpdateDefaultDecoHandler.cs index de31cfd49..2e3620ac4 100644 --- a/ErsatzTV.Application/Scheduling/Commands/UpdateDefaultDecoHandler.cs +++ b/ErsatzTV.Application/Scheduling/Commands/UpdateDefaultDecoHandler.cs @@ -20,7 +20,14 @@ public class UpdateDefaultDecoHandler( int updated = await dbContext.Playouts .Where(p => p.Id == request.PlayoutId) - .ExecuteUpdateAsync(u => u.SetProperty(p => p.DecoId, p => request.DecoId), cancellationToken); + .ExecuteUpdateAsync( + u => u + .SetProperty(p => p.DecoId, p => request.DecoId) + // Bump the shared Playout.Version so any open alternate-schedule/template editor tab + // (which round-trips this same token) detects the deco change and reloads (#253 §7a). + // Bulk ExecuteUpdate bypasses change-tracking, so bump via SetProperty, not Version++. + .SetProperty(p => p.Version, p => p.Version + 1), + cancellationToken); // Changing a playout's default deco only takes effect on a Reset build (deco content is applied // during Reset, and BlockKey has no deco dimension to self-heal) — same stale-until-Reset class 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/MediaCollections/CollectionCustomOrderConcurrencyTests.cs b/ErsatzTV.Tests/Application/MediaCollections/CollectionCustomOrderConcurrencyTests.cs new file mode 100644 index 000000000..a549bcb52 --- /dev/null +++ b/ErsatzTV.Tests/Application/MediaCollections/CollectionCustomOrderConcurrencyTests.cs @@ -0,0 +1,82 @@ +using System.Threading.Channels; +using ErsatzTV.Application; +using ErsatzTV.Application.MediaCollections; +using ErsatzTV.Core; +using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Errors; +using ErsatzTV.Core.Interfaces.Repositories; +using ErsatzTV.Infrastructure.Data; +using ErsatzTV.Tests.Support; +using LanguageExt; +using Microsoft.EntityFrameworkCore; +using NSubstitute; +using NUnit.Framework; +using Shouldly; +using Unit = LanguageExt.Unit; +using static LanguageExt.Prelude; + +namespace ErsatzTV.Tests.Application.MediaCollections; + +/// +/// #253 optimistic-concurrency contract on the Collection custom-order handler (#6): the pre-check +/// 412 (standalone Either after validation, not flattened to 422) and the unconditional Version bump. +/// +[TestFixture] +public class CollectionCustomOrderConcurrencyTests : MediaCollectionHandlerTestBase +{ + private async Task SeedCollectionWithVersion(int id, int version, string name = "Collection") + { + await using TvContext context = Db.CreateContext(); + context.Collections.Add(new Collection + { + Id = id, + Name = name, + Version = version, + MediaItems = [], + CollectionItems = [] + }); + await context.SaveChangesAsync(); + } + + private async Task ReadVersion(int id) + { + await using TvContext context = Db.CreateContext(); + return await context.Collections.Where(c => c.Id == id).Select(c => c.Version).SingleAsync(); + } + + private UpdateCollectionCustomOrderHandler MakeHandler() + { + IMediaCollectionRepository repo = Substitute.For(); + repo.PlayoutIdsUsingCollection(Arg.Any()).Returns([]); + return new UpdateCollectionCustomOrderHandler(Db.Factory, repo, Worker); + } + + private static BaseError LeftOf(Either either) => + either.Match(Left: e => e, Right: _ => throw new AssertionException("Expected a Left result")); + + [Test] + public async Task Stale_If_Match_Should_Fail_Precondition_And_Not_Mutate() + { + await SeedCollectionWithVersion(1, version: 2); + + Either result = await MakeHandler().Handle( + new UpdateCollectionCustomOrder(1, [], Some(1)), + CancellationToken.None); + + LeftOf(result).ShouldBeOfType(); + (await ReadVersion(1)).ShouldBe(2); + } + + [Test] + public async Task Matching_If_Match_Should_Succeed_And_Bump() + { + await SeedCollectionWithVersion(1, version: 2); + + Either result = await MakeHandler().Handle( + new UpdateCollectionCustomOrder(1, [], Some(2)), + CancellationToken.None); + + result.IsRight.ShouldBeTrue(); + (await ReadVersion(1)).ShouldBe(3); + } +} diff --git a/ErsatzTV.Tests/Application/MediaCollections/MultiCollectionConcurrencyTests.cs b/ErsatzTV.Tests/Application/MediaCollections/MultiCollectionConcurrencyTests.cs new file mode 100644 index 000000000..14029bbf2 --- /dev/null +++ b/ErsatzTV.Tests/Application/MediaCollections/MultiCollectionConcurrencyTests.cs @@ -0,0 +1,107 @@ +using System.Threading.Channels; +using ErsatzTV.Application; +using ErsatzTV.Application.MediaCollections; +using ErsatzTV.Core; +using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Errors; +using ErsatzTV.Core.Interfaces.Repositories; +using ErsatzTV.Infrastructure.Data; +using ErsatzTV.Tests.Support; +using LanguageExt; +using Microsoft.EntityFrameworkCore; +using NSubstitute; +using NUnit.Framework; +using Shouldly; +using Unit = LanguageExt.Unit; +using static LanguageExt.Prelude; + +namespace ErsatzTV.Tests.Application.MediaCollections; + +/// +/// #253 optimistic-concurrency contract on the MultiCollection replace handler (#9): pre-check 412 +/// (standalone Either, not flattened to 422), the unconditional Version bump, and the M2 rework of +/// the old SaveChangesAsync() > 0 gate — a name-only change still saves (and bumps) but must +/// NOT rebuild playouts, since the version bump rides the first (name) save while the item save stays +/// gated on real item changes. +/// +[TestFixture] +public class MultiCollectionConcurrencyTests : MediaCollectionHandlerTestBase +{ + private static UpdateMultiCollection Update(int id, string name, Option expectedVersion) => + new(id, name, [], expectedVersion); + + private async Task SeedMultiCollection(int id, int version, string name = "Multi") + { + await using TvContext context = Db.CreateContext(); + context.MultiCollections.Add(new MultiCollection + { + Id = id, + Name = name, + Version = version, + MultiCollectionItems = [], + MultiCollectionSmartItems = [] + }); + await context.SaveChangesAsync(); + } + + private async Task ReadVersion(int id) + { + await using TvContext context = Db.CreateContext(); + return await context.MultiCollections.Where(c => c.Id == id).Select(c => c.Version).SingleAsync(); + } + + private UpdateMultiCollectionHandler MakeHandler(IMediaCollectionRepository repo) => + new(Db.Factory, repo, Worker, SearchTargets); + + private static IMediaCollectionRepository EmptyRepo() + { + IMediaCollectionRepository repo = Substitute.For(); + repo.PlayoutIdsUsingMultiCollection(Arg.Any()).Returns([]); + return repo; + } + + private static BaseError LeftOf(Either either) => + either.Match(Left: e => e, Right: _ => throw new AssertionException("Expected a Left result")); + + [Test] + public async Task Update_Stale_If_Match_Should_Fail_Precondition_And_Not_Mutate() + { + await SeedMultiCollection(1, version: 2); + + Either result = + await MakeHandler(EmptyRepo()).Handle(Update(1, "Renamed", Some(1)), CancellationToken.None); + + LeftOf(result).ShouldBeOfType(); + (await ReadVersion(1)).ShouldBe(2); + } + + [Test] + public async Task Update_Matching_If_Match_Should_Succeed_And_Bump() + { + await SeedMultiCollection(1, version: 2); + + Either result = + await MakeHandler(EmptyRepo()).Handle(Update(1, "Renamed", Some(2)), CancellationToken.None); + + result.IsRight.ShouldBeTrue(); + (await ReadVersion(1)).ShouldBe(3); + } + + [Test] + public async Task Name_Only_Change_Should_Bump_But_Not_Rebuild_Playouts() + { + await SeedMultiCollection(1, version: 1, name: "Before"); + IMediaCollectionRepository repo = EmptyRepo(); + + Either result = + await MakeHandler(repo).Handle(Update(1, "After", None), CancellationToken.None); + + result.IsRight.ShouldBeTrue(); + (await ReadVersion(1)).ShouldBe(2); + + // M2: the version bump rides the first (name) save, so the second (items) save writes nothing → + // no rebuild is enqueued and the search index is not signalled for a name-only edit. + SearchTargets.DidNotReceive().SearchTargetsChanged(); + await repo.DidNotReceive().PlayoutIdsUsingMultiCollection(Arg.Any()); + } +} diff --git a/ErsatzTV.Tests/Application/MediaCollections/ReplacePlaylistItemsHandlerConcurrencyTests.cs b/ErsatzTV.Tests/Application/MediaCollections/ReplacePlaylistItemsHandlerConcurrencyTests.cs new file mode 100644 index 000000000..d1ae0037e --- /dev/null +++ b/ErsatzTV.Tests/Application/MediaCollections/ReplacePlaylistItemsHandlerConcurrencyTests.cs @@ -0,0 +1,155 @@ +using ErsatzTV.Application; +using ErsatzTV.Application.MediaCollections; +using ErsatzTV.Core; +using ErsatzTV.Core.Domain; +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.MediaCollections; + +/// +/// Contract tests for the #253 optimistic-concurrency mechanic on the Playlist aggregate (mirrors +/// ReplaceBlockItemsHandlerConcurrencyTests, 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 Playlist and the losing save silently succeeds instead of +/// mapping to a . +/// +[TestFixture] +public class ReplacePlaylistItemsHandlerConcurrencyTests +{ + private InMemoryTvContext _db = null!; + + [SetUp] + public async Task SetUp() => _db = await InMemoryTvContext.CreateAsync(); + + [TearDown] + public async Task TearDown() => await _db.DisposeAsync(); + + private async Task SeedPlaylistAsync(int version) + { + await using TvContext ctx = _db.CreateContext(); + ctx.Playlists.Add( + new Playlist + { + Id = 1, + PlaylistGroupId = 1, + Name = "Kids", + IsSystem = false, + Version = version, + Items = new List() + }); + await ctx.SaveChangesAsync(); + } + + private static ReplacePlaylistItems Command(Option expectedVersion) => + new( + 1, + "Kids", + new List + { + new(0, CollectionType.Movie, null, null, null, 55, PlaybackOrder.Shuffle, null, false, true) + }, + expectedVersion); + + private async Task ReadVersionAsync() + { + await using TvContext ctx = _db.CreateContext(); + return await ctx.Playlists.Where(p => p.Id == 1).Select(p => p.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 SeedPlaylistAsync(version: 2); + var handler = new ReplacePlaylistItemsHandler(_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.PlaylistItems.CountAsync(i => i.PlaylistId == 1)).ShouldBe(0); + } + + [Test] + public async Task Matching_If_Match_Should_Succeed_And_Bump_Version() + { + await SeedPlaylistAsync(version: 2); + var handler = new ReplacePlaylistItemsHandler(_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 SeedPlaylistAsync(version: 2); + var handler = new ReplacePlaylistItemsHandler(_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 SeedPlaylistAsync(version: 5); + var handler = new ReplacePlaylistItemsHandler(_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 SeedPlaylistAsync(version: 1); + + // Two writers load the same version, then both bump-and-save. The IsConcurrencyToken guard on + // Playlist 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(); + + Playlist winner = await ctxWinner.Playlists.SingleAsync(p => p.Id == 1); + Playlist loser = await ctxLoser.Playlists.SingleAsync(p => p.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/MediaCollections/RerunCollectionHandlerTests.cs b/ErsatzTV.Tests/Application/MediaCollections/RerunCollectionHandlerTests.cs index b1998754d..25e059e0e 100644 --- a/ErsatzTV.Tests/Application/MediaCollections/RerunCollectionHandlerTests.cs +++ b/ErsatzTV.Tests/Application/MediaCollections/RerunCollectionHandlerTests.cs @@ -8,10 +8,12 @@ using ErsatzTV.Core.Interfaces.Repositories; using ErsatzTV.Infrastructure.Data; using ErsatzTV.Tests.Support; using LanguageExt; +using Microsoft.EntityFrameworkCore; using NSubstitute; using NUnit.Framework; using Shouldly; using Unit = LanguageExt.Unit; +using static LanguageExt.Prelude; namespace ErsatzTV.Tests.Application.MediaCollections; @@ -150,7 +152,49 @@ public class RerunCollectionHandlerTests : MediaCollectionHandlerTestBase await context.SaveChangesAsync(); } - private async Task SeedRerunCollection(int id, CollectionType collectionType, string name = "Rerun") + [Test] + public async Task Update_Stale_If_Match_Should_Fail_Precondition() + { + await SeedRerunCollection(1, CollectionType.SmartCollection, version: 2); + await SeedSmartCollection(2); + IMediaCollectionRepository repo = Substitute.For(); + repo.PlayoutIdsUsingRerunCollection(Arg.Any()).Returns([]); + var handler = new UpdateRerunCollectionHandler(Db.Factory, repo, Worker); + + Either result = await handler.Handle( + MakeUpdate(1, CollectionType.SmartCollection, smartCollection: new SmartCollectionViewModel(2, "", "")) + with { ExpectedVersion = Some(1) }, + CancellationToken.None); + + LeftOf(result).ShouldBeOfType(); + (await ReadVersion(1)).ShouldBe(2); + } + + [Test] + public async Task Update_Matching_If_Match_Should_Succeed_And_Bump() + { + await SeedRerunCollection(1, CollectionType.SmartCollection, version: 2); + await SeedSmartCollection(2); + IMediaCollectionRepository repo = Substitute.For(); + repo.PlayoutIdsUsingRerunCollection(Arg.Any()).Returns([]); + var handler = new UpdateRerunCollectionHandler(Db.Factory, repo, Worker); + + Either result = await handler.Handle( + MakeUpdate(1, CollectionType.SmartCollection, smartCollection: new SmartCollectionViewModel(2, "", "")) + with { ExpectedVersion = Some(2) }, + CancellationToken.None); + + RightOf(result); + (await ReadVersion(1)).ShouldBe(3); + } + + private async Task ReadVersion(int id) + { + await using TvContext context = Db.CreateContext(); + return await context.RerunCollections.Where(c => c.Id == id).Select(c => c.Version).SingleAsync(); + } + + private async Task SeedRerunCollection(int id, CollectionType collectionType, string name = "Rerun", int version = 0) { await using TvContext context = Db.CreateContext(); context.RerunCollections.Add(new RerunCollection @@ -158,6 +202,7 @@ public class RerunCollectionHandlerTests : MediaCollectionHandlerTestBase Id = id, Name = name, CollectionType = collectionType, + Version = version, FirstRunPlaybackOrder = PlaybackOrder.Chronological, RerunPlaybackOrder = PlaybackOrder.Chronological }); diff --git a/ErsatzTV.Tests/Application/Playouts/PlayoutHandlerTests.cs b/ErsatzTV.Tests/Application/Playouts/PlayoutHandlerTests.cs index 76a6fd8cc..ebfbd12e4 100644 --- a/ErsatzTV.Tests/Application/Playouts/PlayoutHandlerTests.cs +++ b/ErsatzTV.Tests/Application/Playouts/PlayoutHandlerTests.cs @@ -1,6 +1,7 @@ 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.Errors; @@ -9,6 +10,7 @@ using ErsatzTV.Infrastructure.Data; using ErsatzTV.Tests.Support; using LanguageExt; using MediatR; +using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging.Abstractions; using NSubstitute; using NUnit.Framework; @@ -16,6 +18,7 @@ using Shouldly; using Testably.Abstractions.Testing; using Unit = LanguageExt.Unit; using Channel = System.Threading.Channels.Channel; +using static LanguageExt.Prelude; namespace ErsatzTV.Tests.Application.Playouts; @@ -118,6 +121,176 @@ public class PlayoutHandlerTests LeftOf(result).Value.ShouldContain("must not be empty"); } + // ---- #253 optimistic concurrency (alternate schedules #7 + templates #8, shared Playout.Version) ---- + + private static ReplacePlayoutAlternateSchedule AltItem(int programScheduleId = 10) => + new(0, 0, programScheduleId, [], [], [], false, 1, 1, null, 12, 31, null); + + private static ReplacePlayoutTemplate TemplateItem(int templateId = 20) => + new(0, 0, templateId, null, [], [], [], false, 1, 1, null, 12, 31, null); + + private async Task SeedPlayout(int id, int version, int? programScheduleId = 10) + { + await using TvContext context = _db.CreateContext(); + context.Playouts.Add( + new Playout + { + Id = id, + ChannelId = 1, + ProgramScheduleId = programScheduleId, + Version = version, + Items = [], + ProgramScheduleAlternates = [], + Templates = [] + }); + await context.SaveChangesAsync(); + } + + private async Task ReadPlayoutVersion(int id) + { + await using TvContext context = _db.CreateContext(); + return await context.Playouts.Where(p => p.Id == id).Select(p => p.Version).SingleAsync(); + } + + [Test] + public async Task ReplaceAlternateSchedules_Stale_If_Match_Should_Fail_Precondition_And_Not_Mutate() + { + await SeedPlayout(1, version: 2); + var handler = new ReplacePlayoutAlternateScheduleItemsHandler( + _db.Factory, + _worker, + NullLogger.Instance); + + Either result = await handler.Handle( + new ReplacePlayoutAlternateScheduleItems(1, [AltItem()], Some(1)), + CancellationToken.None); + + // Standalone-Either pre-check (not via Apply) preserves the subtype → 412 rather than a flattened 422. + LeftOf(result).ShouldBeOfType(); + (await ReadPlayoutVersion(1)).ShouldBe(2); + } + + [Test] + public async Task ReplaceAlternateSchedules_Matching_If_Match_Should_Succeed_And_Bump() + { + await SeedPlayout(1, version: 2); + var handler = new ReplacePlayoutAlternateScheduleItemsHandler( + _db.Factory, + _worker, + NullLogger.Instance); + + Either result = await handler.Handle( + new ReplacePlayoutAlternateScheduleItems(1, [AltItem()], Some(2)), + CancellationToken.None); + + result.IsRight.ShouldBeTrue(); + (await ReadPlayoutVersion(1)).ShouldBe(3); + } + + [Test] + public async Task ReplaceAlternateSchedules_Absent_If_Match_Should_Force_Write_And_Bump() + { + await SeedPlayout(1, version: 2); + var handler = new ReplacePlayoutAlternateScheduleItemsHandler( + _db.Factory, + _worker, + NullLogger.Instance); + + Either result = await handler.Handle( + new ReplacePlayoutAlternateScheduleItems(1, [AltItem()], None), + CancellationToken.None); + + result.IsRight.ShouldBeTrue(); + (await ReadPlayoutVersion(1)).ShouldBe(3); + } + + [Test] + public async Task ReplaceTemplates_Stale_If_Match_Should_Fail_Precondition_And_Not_Mutate() + { + await SeedPlayout(1, version: 2); + var handler = new ReplacePlayoutTemplateItemsHandler( + _db.Factory, + NullLogger.Instance); + + Option result = await handler.Handle( + new ReplacePlayoutTemplateItems(1, [TemplateItem()], Some(1)), + CancellationToken.None); + + // Some(PreconditionFailedError): the pre-check escapes the handler's catch(Exception) as a 412 + // rather than being reshaped into a bare BaseError → 422 (§9/H1). + result.IfNone(() => throw new AssertionException("Expected a Some(error)")) + .ShouldBeOfType(); + (await ReadPlayoutVersion(1)).ShouldBe(2); + } + + [Test] + public async Task ReplaceTemplates_Matching_If_Match_Should_Succeed_And_Bump() + { + await SeedPlayout(1, version: 2); + var handler = new ReplacePlayoutTemplateItemsHandler( + _db.Factory, + NullLogger.Instance); + + Option result = await handler.Handle( + new ReplacePlayoutTemplateItems(1, [TemplateItem()], Some(2)), + CancellationToken.None); + + result.IsNone.ShouldBeTrue(); + (await ReadPlayoutVersion(1)).ShouldBe(3); + } + + [Test] + public async Task Racing_Playout_Save_Should_Map_Concurrency_Failure_To_412() + { + await SeedPlayout(1, version: 1); + + // Backstop mechanic on the Playout token: two writers load the same version and both bump-and-save; + // IsConcurrencyToken on Playout keys the loser's UPDATE on the original version → 0 rows → + // DbUpdateConcurrencyException → PreconditionFailedError. Non-vacuous: drop .IsConcurrencyToken() + // on Playout and the loser silently overwrites the winner instead. + await using TvContext ctxWinner = _db.CreateContext(); + await using TvContext ctxLoser = _db.CreateContext(); + + Playout winner = await ctxWinner.Playouts.SingleAsync(p => p.Id == 1); + Playout loser = await ctxLoser.Playouts.SingleAsync(p => p.Id == 1); + + winner.Version++; + (await ctxWinner.SaveChangesWithConcurrencyGuard(CancellationToken.None)).IsRight.ShouldBeTrue(); + + loser.Version++; + Either loserResult = await ctxLoser.SaveChangesWithConcurrencyGuard(CancellationToken.None); + + loserResult.Match(Right: _ => null, Left: e => e).ShouldBeOfType(); + (await ReadPlayoutVersion(1)).ShouldBe(2); + } + + [Test] + public async Task SaveChangesForcingVersion_Should_Force_Write_Past_A_Concurrent_Bump_Without_Reverting_It() + { + await SeedPlayout(1, version: 1); + + await using TvContext ctxSettings = _db.CreateContext(); + await using TvContext ctxBumper = _db.CreateContext(); + + Playout settings = await ctxSettings.Playouts.SingleAsync(p => p.Id == 1); + Playout bumper = await ctxBumper.Playouts.SingleAsync(p => p.Id == 1); + + // A replace-all editor bumps the version in another context after the settings writer loaded. + bumper.Version++; + await ctxBumper.SaveChangesAsync(); + + // The non-participating settings writer changes a root scalar against the now-stale version. Plain + // SaveChangesAsync would throw DbUpdateConcurrencyException (proven by the racing test above); the + // forcing helper must land the write instead (Phase-1 force-write) and NOT revert the bump. + settings.DailyRebuildTime = TimeSpan.FromHours(3); + await ctxSettings.SaveChangesForcingVersion(CancellationToken.None); + + await using TvContext verify = _db.CreateContext(); + Playout result = await verify.Playouts.SingleAsync(p => p.Id == 1); + result.DailyRebuildTime.ShouldBe(TimeSpan.FromHours(3)); + result.Version.ShouldBe(2); + } + private static BaseError LeftOf(Either either) => either.Match(Left: e => e, Right: _ => throw new AssertionException("Expected a Left result")); diff --git a/ErsatzTV.Tests/Application/ProgramSchedules/ReplaceProgramScheduleItemsHandlerConcurrencyTests.cs b/ErsatzTV.Tests/Application/ProgramSchedules/ReplaceProgramScheduleItemsHandlerConcurrencyTests.cs new file mode 100644 index 000000000..d665cdf3b --- /dev/null +++ b/ErsatzTV.Tests/Application/ProgramSchedules/ReplaceProgramScheduleItemsHandlerConcurrencyTests.cs @@ -0,0 +1,231 @@ +using System.Threading.Channels; +using ErsatzTV.Application; +using ErsatzTV.Application.ProgramSchedules; +using ErsatzTV.Core; +using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Errors; +using ErsatzTV.Core.Scheduling; +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.ProgramSchedules; + +/// +/// Contract tests for the #253 optimistic-concurrency mechanic on the ProgramSchedule / schedule-items +/// aggregate: the handler pre-check (stale If-Match → 412, before the positional reconcile runs — so the +/// item rows AND persisted fill-group/shuffle state are left untouched), the force-write path (no +/// If-Match), the unconditional Version bump on every save (including a no-op same-items PUT-back where +/// only child rows change), 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 ProgramSchedule and the losing save silently succeeds instead +/// of mapping to a . +/// +[TestFixture] +public class ReplaceProgramScheduleItemsHandlerConcurrencyTests +{ + private InMemoryTvContext _db = null!; + private ChannelWriter _worker = null!; + + [SetUp] + public async Task SetUp() + { + _db = await InMemoryTvContext.CreateAsync(); + _worker = System.Threading.Channels.Channel.CreateUnbounded().Writer; + } + + [TearDown] + public async Task TearDown() => await _db.DisposeAsync(); + + // Seeds a schedule (Id=1) with a single One/SearchQuery item (Id=1) and a persisted fill-group + // enumerator state pointing at that item, so the stale-If-Match test can prove the reconcile never ran. + private async Task SeedScheduleAsync(int version) + { + await using TvContext ctx = _db.CreateContext(); + ctx.ProgramSchedules.Add( + new ProgramSchedule + { + Id = 1, + Name = "Concurrency", + Version = version, + Items = new List + { + new ProgramScheduleItemOne + { + Id = 1, + Index = 0, + CollectionType = CollectionType.SearchQuery, + SearchTitle = "a", + SearchQuery = "a", + PlaybackOrder = PlaybackOrder.Shuffle, + GuideMode = GuideMode.Normal + } + }, + Playouts = [], + ProgramScheduleAlternates = [] + }); + await ctx.SaveChangesAsync(); + + ctx.Add(new PlayoutScheduleItemFillGroupIndex + { + PlayoutId = 1, + ProgramScheduleItemId = 1, + EnumeratorState = new CollectionEnumeratorState { Seed = 12345, Index = 7 } + }); + await ctx.SaveChangesAsync(); + } + + private static ReplaceProgramScheduleItems Command( + Option expectedVersion, + List? items = null) => + new(1, items ?? [MakeItem(0, PlayoutMode.One, "a")], expectedVersion); + + private async Task ReadVersionAsync() + { + await using TvContext ctx = _db.CreateContext(); + return await ctx.ProgramSchedules.Where(s => s.Id == 1).Select(s => s.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 SeedScheduleAsync(version: 2); + var handler = new ReplaceProgramScheduleItemsHandler(_db.Factory, _worker); + + // An empty item list WOULD delete the existing item (and cascade its fill-group state) if the + // reconcile ran. A stale If-Match must reject before that, leaving everything untouched. + Either> result = + await handler.Handle(Command(Some(1), []), CancellationToken.None); + + LeftOrNull(result).ShouldBeOfType(); + + (await ReadVersionAsync()).ShouldBe(2); + await using TvContext ctx = _db.CreateContext(); + (await ctx.ProgramScheduleItems.CountAsync(i => i.ProgramScheduleId == 1)).ShouldBe(1); + + // The reconcile never ran: the fill-group enumerator state is exactly as seeded. + PlayoutScheduleItemFillGroupIndex fillGroup = await ctx.Set() + .Include(x => x.EnumeratorState) + .SingleAsync(); + fillGroup.ProgramScheduleItemId.ShouldBe(1); + fillGroup.EnumeratorState.Seed.ShouldBe(12345); + fillGroup.EnumeratorState.Index.ShouldBe(7); + } + + [Test] + public async Task Matching_If_Match_Should_Succeed_And_Bump_Version() + { + await SeedScheduleAsync(version: 2); + var handler = new ReplaceProgramScheduleItemsHandler(_db.Factory, _worker); + + 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 SeedScheduleAsync(version: 2); + var handler = new ReplaceProgramScheduleItemsHandler(_db.Factory, _worker); + + // 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 NoOp_Save_Should_Bump_Version_Even_With_Only_Child_Changes() + { + await SeedScheduleAsync(version: 5); + var handler = new ReplaceProgramScheduleItemsHandler(_db.Factory, _worker); + + // Same content twice: this handler saves with only CHILD changes and no root-scalar change, so the + // unconditional bump (M1) must still rotate the version each time, otherwise a no-op PUT-back would + // neither fire the token nor 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 SeedScheduleAsync(version: 1); + + // Two writers load the same version, then both bump-and-save. The IsConcurrencyToken guard on + // ProgramSchedule 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(); + + ProgramSchedule winner = await ctxWinner.ProgramSchedules.SingleAsync(s => s.Id == 1); + ProgramSchedule loser = await ctxLoser.ProgramSchedules.SingleAsync(s => s.Id == 1); + + winner.Version++; + Either winnerResult = await ctxWinner.SaveChangesWithConcurrencyGuard(CancellationToken.None); + winnerResult.IsRight.ShouldBeTrue(); + + loser.Version++; + Either loserResult = await ctxLoser.SaveChangesWithConcurrencyGuard(CancellationToken.None); + + loserResult.Match(Right: _ => null, Left: e => e).ShouldBeOfType(); + + // Non-vacuous: the winner's write stuck at exactly its +1; the loser did not overwrite it. + (await ReadVersionAsync()).ShouldBe(2); + } + + private static ReplaceProgramScheduleItem MakeItem(int index, PlayoutMode mode, string searchQuery) => + new( + index, + StartType.Dynamic, + StartTime: null, + FixedStartTimeBehavior: null, + mode, + CollectionType.SearchQuery, + CollectionId: null, + MultiCollectionId: null, + SmartCollectionId: null, + RerunCollectionId: null, + MediaItemId: null, + PlaylistId: null, + SearchTitle: searchQuery, + SearchQuery: searchQuery, + PlaybackOrder.Shuffle, + MarathonGroupBy.None, + MarathonShuffleGroups: false, + MarathonShuffleItems: false, + MarathonBatchSize: null, + FillWithGroupMode.None, + MultipleMode.Count, + MultipleCount: "1", + PlayoutDuration: null, + TailMode.None, + DiscardToFillAttempts: null, + CustomTitle: null, + GuideMode.Normal, + PreRollFillerId: null, + MidRollFillerId: null, + PostRollFillerId: null, + TailFillerId: null, + FallbackFillerId: null, + WatermarkIds: [], + GraphicsElementIds: [], + PreferredAudioLanguageCode: null, + PreferredAudioTitle: null, + PreferredSubtitleLanguageCode: null, + SubtitleMode: null); +} 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/ChannelControllerTests.cs b/ErsatzTV.Tests/Controllers/ChannelControllerTests.cs index 5e1deb4e3..f1f12b89f 100644 --- a/ErsatzTV.Tests/Controllers/ChannelControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/ChannelControllerTests.cs @@ -454,7 +454,8 @@ public class ChannelControllerTests null, null, null, - null); + null, + 0); private static ChannelViewModel MakeVm(int id) => new( diff --git a/ErsatzTV.Tests/Controllers/CollectionControllerTests.cs b/ErsatzTV.Tests/Controllers/CollectionControllerTests.cs index fff04b2be..9914e4e2d 100644 --- a/ErsatzTV.Tests/Controllers/CollectionControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/CollectionControllerTests.cs @@ -8,6 +8,7 @@ using ErsatzTV.Core.Domain; 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 +29,11 @@ public class CollectionControllerTests public void SetUp() { _mediator = Substitute.For(); - _controller = new CollectionController(_mediator); + _controller = new CollectionController(_mediator) + { + // Real HttpContext so the #253 ETag/If-Match concurrency headers can be read/written. + ControllerContext = new ControllerContext { HttpContext = new DefaultHttpContext() } + }; } [Test] diff --git a/ErsatzTV.Tests/Controllers/DecoControllerTests.cs b/ErsatzTV.Tests/Controllers/DecoControllerTests.cs index 38068d4ac..e96eae117 100644 --- a/ErsatzTV.Tests/Controllers/DecoControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/DecoControllerTests.cs @@ -152,7 +152,7 @@ public class DecoControllerTests null, null, null, - new ErsatzTV.Application.MediaCollections.PlaylistViewModel(9, 3, "Idents", false), + new ErsatzTV.Application.MediaCollections.PlaylistViewModel(9, 3, "Idents", false, 1), DecoBreakPlacement.BlockStart) ]); _mediator.Send(Arg.Any(), Arg.Any()) diff --git a/ErsatzTV.Tests/Controllers/DecoTemplateControllerTests.cs b/ErsatzTV.Tests/Controllers/DecoTemplateControllerTests.cs index 6ebfe6754..e5ed3b39c 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,10 @@ public class DecoTemplateControllerTests _mediator.Send(Arg.Any(), Arg.Any()) .Returns(Option.Some(MakeDecoTemplate(4, 7, "Morning"))); _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Right>([])); + .Returns(Right>( + [MakeItem(10, "Morning Bumper", TimeSpan.FromHours(6), TimeSpan.FromHours(7))])); _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(new List - { - MakeItem(10, "Morning Bumper", TimeSpan.FromHours(6), TimeSpan.FromHours(7)) - }); + .Returns([MakeItem(10, "Morning Bumper", TimeSpan.FromHours(6), TimeSpan.FromHours(7))]); IActionResult result = await _controller.Replace( 4, @@ -318,11 +323,100 @@ 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>([])); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(new List()); + + 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>([])); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(new List()); + + 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/MultiCollectionControllerTests.cs b/ErsatzTV.Tests/Controllers/MultiCollectionControllerTests.cs index d40215341..d9e491943 100644 --- a/ErsatzTV.Tests/Controllers/MultiCollectionControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/MultiCollectionControllerTests.cs @@ -7,6 +7,7 @@ using ErsatzTV.Core.Api.MediaCollections; using ErsatzTV.Core.Domain; using LanguageExt; using MediatR; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Routing; using NSubstitute; @@ -24,7 +25,11 @@ public class MultiCollectionControllerTests public void SetUp() { _mediator = Substitute.For(); - _controller = new MultiCollectionController(_mediator); + _controller = new MultiCollectionController(_mediator) + { + // Real HttpContext so the #253 ETag/If-Match concurrency headers can be read/written. + ControllerContext = new ControllerContext { HttpContext = new DefaultHttpContext() } + }; } private MultiCollectionController _controller = null!; diff --git a/ErsatzTV.Tests/Controllers/PlaylistControllerTests.cs b/ErsatzTV.Tests/Controllers/PlaylistControllerTests.cs index 07dc7a086..be24d0f38 100644 --- a/ErsatzTV.Tests/Controllers/PlaylistControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/PlaylistControllerTests.cs @@ -7,8 +7,10 @@ using ErsatzTV.Controllers.Api.Requests; using ErsatzTV.Core; using ErsatzTV.Core.Api.MediaCollections; using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Errors; using LanguageExt; using MediatR; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Routing; using NSubstitute; @@ -26,7 +28,12 @@ public class PlaylistControllerTests public void SetUp() { _mediator = Substitute.For(); - _controller = new PlaylistController(_mediator); + _controller = new PlaylistController(_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() } + }; } private PlaylistController _controller = null!; @@ -204,7 +211,7 @@ public class PlaylistControllerTests public async Task GetById_Should_Return_200_For_Some() { _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false))); + .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false, 1))); IActionResult result = await _controller.GetById(4, CancellationToken.None); @@ -226,7 +233,7 @@ public class PlaylistControllerTests public async Task GetItems_Should_Return_200_And_Flatten_Names() { _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false))); + .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false, 1))); _mediator.Send(Arg.Any(), Arg.Any()) .Returns(new List { @@ -280,11 +287,24 @@ public class PlaylistControllerTests await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); } + [Test] + public async Task GetItems_Should_Set_ETag_From_Playlist_Version() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false, 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 Create_Should_Return_201_And_Map_Request() { _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Right(new PlaylistViewModel(9, 1, "Kids", false))); + .Returns(Right(new PlaylistViewModel(9, 1, "Kids", false, 1))); IActionResult result = await _controller.Create( new CreatePlaylistRequest(1, "Kids"), @@ -313,8 +333,12 @@ public class PlaylistControllerTests [Test] public async Task Update_Should_Return_200_With_Items_And_Map_Request_By_Array_Order() { + // Existence pre-check reads version 1; the post-save re-query reads the bumped version 2 — + // the response ETag must carry the refreshed value (issue #253). _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false))); + .Returns( + Option.Some(new PlaylistViewModel(4, 1, "Kids", false, 1)), + Option.Some(new PlaylistViewModel(4, 1, "Kids", false, 2))); _mediator.Send(Arg.Any(), Arg.Any()) .Returns(Right>(new List { @@ -331,6 +355,22 @@ public class PlaylistControllerTests false, true) })); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(new List + { + new( + 100, + 0, + CollectionType.Movie, + null, + null, + null, + new NamedMediaItemViewModel(55, "The Movie"), + PlaybackOrder.Shuffle, + null, + false, + true) + }); IActionResult result = await _controller.Update( 4, @@ -346,6 +386,8 @@ public class PlaylistControllerTests result.ShouldBeOfType().Value.ShouldBeOfType>().Count .ShouldBe(1); + // On success the response carries the refreshed playlist's ETag. + _controller.Response.Headers.ETag.ToString().ShouldBe("\"2\""); await _mediator.Received(1).Send( Arg.Is(c => c.PlaylistId == 4 && @@ -358,6 +400,81 @@ public class PlaylistControllerTests Arg.Any()); } + [Test] + public async Task Update_Should_Return_400_On_Malformed_If_Match() + { + _controller.Request.Headers.IfMatch = "not-an-etag"; + + IActionResult result = await _controller.Update( + 4, + new ReplacePlaylistRequest("Kids", new List()), + CancellationToken.None); + + result.ShouldBeOfType(); + await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + } + + [Test] + public async Task Update_Should_Thread_If_Match_Version_Into_Command() + { + _controller.Request.Headers.IfMatch = "\"3\""; + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false, 3))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Right>(new List())); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(new List()); + + IActionResult result = await _controller.Update( + 4, + new ReplacePlaylistRequest("Kids", new List()), + CancellationToken.None); + + result.ShouldBeOfType(); + await _mediator.Received(1).Send( + Arg.Is(c => c.ExpectedVersion == Option.Some(3)), + Arg.Any()); + } + + [Test] + public async Task Update_Without_If_Match_Should_Force_Write() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false, 1))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Right>(new List())); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(new List()); + + await _controller.Update( + 4, + new ReplacePlaylistRequest("Kids", new List()), + CancellationToken.None); + + await _mediator.Received(1).Send( + Arg.Is(c => c.ExpectedVersion == Option.None), + Arg.Any()); + } + + [Test] + public async Task Update_Should_Return_412_On_Precondition_Failed() + { + _controller.Request.Headers.IfMatch = "\"2\""; + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false, 5))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Left>(new PreconditionFailedError("stale"))); + + IActionResult result = await _controller.Update( + 4, + new ReplacePlaylistRequest("Kids", new List()), + CancellationToken.None); + + var objectResult = result.ShouldBeOfType(); + objectResult.StatusCode.ShouldBe(StatusCodes.Status412PreconditionFailed); + } + [Test] public async Task Update_Should_Return_404_When_Playlist_Missing() { @@ -377,7 +494,7 @@ public class PlaylistControllerTests public async Task Update_Should_Return_422_On_Validation_Error() { _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false))); + .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false, 1))); _mediator.Send(Arg.Any(), Arg.Any()) .Returns(Left>(BaseError.New("bad item"))); @@ -393,7 +510,7 @@ public class PlaylistControllerTests public async Task Update_Should_Return_422_On_System_Playlist_And_Not_Replace_Items() { _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Option.Some(new PlaylistViewModel(4, 1, "System", true))); + .Returns(Option.Some(new PlaylistViewModel(4, 1, "System", true, 1))); IActionResult result = await _controller.Update( 4, @@ -410,7 +527,7 @@ public class PlaylistControllerTests public async Task Delete_Should_Return_204_On_Success() { _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false))); + .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false, 1))); _mediator.Send(Arg.Any(), Arg.Any()) .Returns(Option.None); @@ -438,7 +555,7 @@ public class PlaylistControllerTests public async Task Delete_Should_Return_422_On_System_Playlist() { _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Option.Some(new PlaylistViewModel(4, 1, "System", true))); + .Returns(Option.Some(new PlaylistViewModel(4, 1, "System", true, 1))); _mediator.Send(Arg.Any(), Arg.Any()) .Returns(Option.Some(BaseError.New("Cannot delete system (generated) playlist"))); @@ -451,7 +568,7 @@ public class PlaylistControllerTests public async Task AddItems_Should_Return_204_And_Map_Request() { _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false))); + .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false, 1))); _mediator.Send(Arg.Any(), Arg.Any()) .Returns(Right(Unit.Default)); @@ -488,7 +605,7 @@ public class PlaylistControllerTests public async Task AddItems_Should_Return_422_On_System_Playlist() { _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Option.Some(new PlaylistViewModel(4, 1, "System", true))); + .Returns(Option.Some(new PlaylistViewModel(4, 1, "System", true, 1))); _mediator.Send(Arg.Any(), Arg.Any()) .Returns(Left(BaseError.New("Cannot add items to system (generated) playlist"))); @@ -504,7 +621,7 @@ public class PlaylistControllerTests public async Task AddItems_Should_Return_422_On_Validation_Error() { _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false))); + .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false, 1))); _mediator.Send(Arg.Any(), Arg.Any()) .Returns(Left(BaseError.New("Movie does not exist"))); diff --git a/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs b/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs index 1b3e25c3f..7eb4e769d 100644 --- a/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs @@ -17,6 +17,7 @@ using ErsatzTV.Core.Interfaces.Locking; using ErsatzTV.Core.Scheduling; using LanguageExt; using MediatR; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Routing; using NSubstitute; @@ -39,7 +40,11 @@ public class PlayoutControllerTests { _mediator = Substitute.For(); _entityLocker = Substitute.For(); - _controller = new PlayoutController(_mediator, _entityLocker); + _controller = new PlayoutController(_mediator, _entityLocker) + { + // Real HttpContext so the #253 ETag/If-Match concurrency headers can be read/written. + ControllerContext = new ControllerContext { HttpContext = new DefaultHttpContext() } + }; } [Test] @@ -1131,7 +1136,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()) @@ -1270,16 +1275,16 @@ public class PlayoutControllerTests new(0, programScheduleId, [], [], [], false, 1, 1, null, 12, 31, null); private static ProgramScheduleViewModel MakeScheduleVm(int id) => - new(id, $"Schedule {id}", false, false, false, false, FixedStartTimeBehavior.Strict); + new(id, $"Schedule {id}", false, false, false, false, FixedStartTimeBehavior.Strict, 0); 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, [], [], @@ -1307,7 +1312,8 @@ public class PlayoutControllerTests null, new PlayoutBuildStatus(), null, - null); + null, + 0); private static PlayoutResponseModel ToResponse(PlayoutNameViewModel vm, bool isLocked = false) => PlayoutResponseModel.From( diff --git a/ErsatzTV.Tests/Controllers/RerunCollectionControllerTests.cs b/ErsatzTV.Tests/Controllers/RerunCollectionControllerTests.cs index 9a6430775..6f4cf53ab 100644 --- a/ErsatzTV.Tests/Controllers/RerunCollectionControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/RerunCollectionControllerTests.cs @@ -7,6 +7,7 @@ using ErsatzTV.Core.Api.MediaCollections; using ErsatzTV.Core.Domain; using LanguageExt; using MediatR; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Routing; using NSubstitute; @@ -24,7 +25,11 @@ public class RerunCollectionControllerTests public void SetUp() { _mediator = Substitute.For(); - _controller = new RerunCollectionController(_mediator); + _controller = new RerunCollectionController(_mediator) + { + // Real HttpContext so the #253 ETag/If-Match concurrency headers can be read/written. + ControllerContext = new ControllerContext { HttpContext = new DefaultHttpContext() } + }; } private RerunCollectionController _controller = null!; diff --git a/ErsatzTV.Tests/Controllers/ScheduleControllerTests.cs b/ErsatzTV.Tests/Controllers/ScheduleControllerTests.cs index e09792cc3..a0494db99 100644 --- a/ErsatzTV.Tests/Controllers/ScheduleControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/ScheduleControllerTests.cs @@ -10,6 +10,7 @@ using ErsatzTV.Core.Errors; using ErsatzTV.Core.Scheduling; using LanguageExt; using MediatR; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Routing; using NSubstitute; @@ -30,7 +31,12 @@ public class ScheduleControllerTests public void SetUp() { _mediator = Substitute.For(); - _controller = new ScheduleController(_mediator); + _controller = new ScheduleController(_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] @@ -253,6 +259,10 @@ public class ScheduleControllerTests List items = [MakeOneItem(21), MakeOneItem(22)]; _mediator.Send(Arg.Any(), Arg.Any()) .Returns(Right>(items)); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(MakeSchedule(4, "Daily"))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(items); IActionResult result = await _controller.ReplaceItems( 4, @@ -271,6 +281,94 @@ public class ScheduleControllerTests Arg.Any()); } + [Test] + public async Task GetItems_Should_Set_ETag_From_Schedule_Version() + { + var response = new ProgramScheduleItemsWithDurationViewModel([], null); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(MakeSchedule(4, "Daily", version: 9))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(response); + + await _controller.GetItems(4, CancellationToken.None); + + _controller.Response.Headers.ETag.ToString().ShouldBe("\"9\""); + } + + [Test] + public async Task ReplaceItems_Should_Return_400_On_Malformed_If_Match() + { + _controller.Request.Headers.IfMatch = "not-an-etag"; + + IActionResult result = await _controller.ReplaceItems( + 4, + new ReplaceScheduleItemsRequest([MakeItemRequest(PlayoutMode.One)]), + CancellationToken.None); + + result.ShouldBeOfType(); + await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + } + + [Test] + public async Task ReplaceItems_Should_Thread_If_Match_Version_Into_Command_And_Set_New_ETag() + { + _controller.Request.Headers.IfMatch = "\"3\""; + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Right>([MakeOneItem(21)])); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(MakeSchedule(4, "Daily", version: 4))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns([MakeOneItem(21)]); + + IActionResult result = await _controller.ReplaceItems( + 4, + new ReplaceScheduleItemsRequest([MakeItemRequest(PlayoutMode.One)]), + CancellationToken.None); + + result.ShouldBeOfType(); + // On success the response carries the refreshed schedule'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 ReplaceItems_Without_If_Match_Should_Force_Write() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Right>([MakeOneItem(21)])); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(MakeSchedule(4, "Daily", version: 1))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns([MakeOneItem(21)]); + + await _controller.ReplaceItems( + 4, + new ReplaceScheduleItemsRequest([MakeItemRequest(PlayoutMode.One)]), + CancellationToken.None); + + await _mediator.Received(1).Send( + Arg.Is(c => c.ExpectedVersion == Option.None), + Arg.Any()); + } + + [Test] + public async Task ReplaceItems_Should_Return_412_On_Precondition_Failed() + { + _controller.Request.Headers.IfMatch = "\"2\""; + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Left>(new PreconditionFailedError("stale"))); + + IActionResult result = await _controller.ReplaceItems( + 4, + new ReplaceScheduleItemsRequest([MakeItemRequest(PlayoutMode.One)]), + CancellationToken.None); + + var objectResult = result.ShouldBeOfType(); + objectResult.StatusCode.ShouldBe(StatusCodes.Status412PreconditionFailed); + } + [Test] public async Task DeleteItem_Should_Return_204_And_Map_Route_Ids() { @@ -343,8 +441,8 @@ public class ScheduleControllerTests PreferredSubtitleLanguageCode: null, SubtitleMode: null); - private static ProgramScheduleViewModel MakeSchedule(int id, string name) => - new(id, name, true, true, false, false, FixedStartTimeBehavior.Flexible); + private static ProgramScheduleViewModel MakeSchedule(int id, string name, int version = 0) => + new(id, name, true, true, false, false, FixedStartTimeBehavior.Flexible, version); private static ProgramScheduleItemOneViewModel MakeOneItem(int id) => new( diff --git a/ErsatzTV.Tests/Controllers/TemplateControllerTests.cs b/ErsatzTV.Tests/Controllers/TemplateControllerTests.cs index a56f47017..ffe27ace4 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,10 @@ public class TemplateControllerTests _mediator.Send(Arg.Any(), Arg.Any()) .Returns(Option.Some(MakeTemplate(4, 7, "Morning"))); _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Right>([])); + .Returns(Right>( + [MakeItem(10, "Cartoons", TimeSpan.FromHours(6), 60)])); _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(new List - { - MakeItem(10, "Cartoons", TimeSpan.FromHours(6), 60) - }); + .Returns([MakeItem(10, "Cartoons", TimeSpan.FromHours(6), 60)]); IActionResult result = await _controller.Replace( 4, @@ -305,7 +310,96 @@ 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>([])); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(new List()); + + 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>([])); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(new List()); + + 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 +453,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/CollectionController.cs b/ErsatzTV/Controllers/Api/CollectionController.cs index 67e9b77d1..81031f2e7 100644 --- a/ErsatzTV/Controllers/Api/CollectionController.cs +++ b/ErsatzTV/Controllers/Api/CollectionController.cs @@ -49,6 +49,15 @@ public class CollectionController(IMediator mediator) : ControllerBase int clampedPageNum = Math.Max(0, pageNum); int clampedPageSize = Math.Clamp(pageSize, 1, 100); + // The items GET is the reorder editor's load endpoint: emit the collection's version as the + // concurrency ETag (issue #253). The response carries children, so read the root separately. + Option maybeCollection = + await mediator.Send(new GetCollectionById(id), cancellationToken); + foreach (MediaCollectionViewModel collection in maybeCollection) + { + ConcurrencyHeaders.SetETag(Response, collection.Version); + } + Either result = await mediator.Send( new GetCollectionItems(id, clampedPageNum, clampedPageSize), cancellationToken); @@ -105,13 +114,21 @@ public class CollectionController(IMediator mediator) : ControllerBase "UseCustomPlaybackOrder on the collection for this order to take effect.")] [EndpointGroupName("general")] [ProducesResponseType(StatusCodes.Status204NoContent)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status400BadRequest)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status412PreconditionFailed)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task UpdateCustomOrder( int id, [Required] [FromBody] UpdateCollectionCustomOrderRequest request, CancellationToken cancellationToken) { + IfMatchCondition ifMatch = ConcurrencyHeaders.ParseIfMatch(Request); + if (ifMatch.Kind is IfMatchKind.Malformed) + { + return ConcurrencyHeaders.MalformedIfMatchProblem(); + } + Option maybeCollection = await mediator.Send(new GetCollectionById(id), cancellationToken); if (maybeCollection.IsNone) @@ -119,8 +136,18 @@ public class CollectionController(IMediator mediator) : ControllerBase return ApiResults.NotFoundProblem(); } - Either result = await mediator.Send(request.ToCommand(id), cancellationToken); - return result.ToDeletedResult(); + Either result = + await mediator.Send(request.ToCommand(id, ifMatch.ExpectedVersion), cancellationToken); + return await result.Match( + Left: error => Task.FromResult(error.ToErrorResult()), + Right: async _ => + { + // Emit the new ETag on the bodyless 204 so a same-tab second save doesn't 412 (#253). + Option refreshed = + await mediator.Send(new GetCollectionById(id), cancellationToken); + ConcurrencyHeaders.SetETag(Response, refreshed.Map(c => c.Version).IfNone(0)); + return (IActionResult)new NoContentResult(); + }); } [HttpDelete("/api/collections/{id:int}")] diff --git a/ErsatzTV/Controllers/Api/DecoTemplateController.cs b/ErsatzTV/Controllers/Api/DecoTemplateController.cs index 82a316239..53c8e122e 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,27 @@ 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 _ => { + // Reload the root (version) FIRST, then the items, so the emitted ETag is never newer than + // the returned items (issue #253 fail-safe ordering; matches BlockController). 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/MultiCollectionController.cs b/ErsatzTV/Controllers/Api/MultiCollectionController.cs index 6fa0b2e6e..d666d7586 100644 --- a/ErsatzTV/Controllers/Api/MultiCollectionController.cs +++ b/ErsatzTV/Controllers/Api/MultiCollectionController.cs @@ -48,6 +48,13 @@ public class MultiCollectionController(IMediator mediator) : ControllerBase { Option result = await mediator.Send(new GetMultiCollectionById(id), cancellationToken); + + // This by-id GET is the editor's load endpoint: emit the concurrency ETag (issue #253). + foreach (MultiCollectionViewModel vm in result) + { + ConcurrencyHeaders.SetETag(Response, vm.Version); + } + return result.Map(ProjectToResponseModel).ToGetResult(); } @@ -74,14 +81,23 @@ public class MultiCollectionController(IMediator mediator) : ControllerBase [EndpointSummary("Update a multi collection")] [EndpointGroupName("general")] [ProducesResponseType(typeof(MultiCollectionResponseModel), 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 Update( int id, [Required] [FromBody] UpdateMultiCollectionRequest request, CancellationToken cancellationToken) { - Either result = await mediator.Send(request.ToCommand(id), cancellationToken); + IfMatchCondition ifMatch = ConcurrencyHeaders.ParseIfMatch(Request); + if (ifMatch.Kind is IfMatchKind.Malformed) + { + return ConcurrencyHeaders.MalformedIfMatchProblem(); + } + + Either result = + await mediator.Send(request.ToCommand(id, ifMatch.ExpectedVersion), cancellationToken); return await result.Match( Left: error => Task.FromResult(error.ToErrorResult()), Right: async _ => @@ -89,7 +105,12 @@ public class MultiCollectionController(IMediator mediator) : ControllerBase Option multiCollection = await mediator.Send(new GetMultiCollectionById(id), cancellationToken); return multiCollection.Match( - Some: vm => (IActionResult)new OkObjectResult(ProjectToResponseModel(vm)), + Some: vm => + { + // Emit the new ETag so a same-tab second save doesn't 412 against its own write (#253). + ConcurrencyHeaders.SetETag(Response, vm.Version); + return (IActionResult)new OkObjectResult(ProjectToResponseModel(vm)); + }, None: () => ApiResults.NotFoundProblem()); }); } diff --git a/ErsatzTV/Controllers/Api/PlaylistController.cs b/ErsatzTV/Controllers/Api/PlaylistController.cs index f93f63ced..e9d165c88 100644 --- a/ErsatzTV/Controllers/Api/PlaylistController.cs +++ b/ErsatzTV/Controllers/Api/PlaylistController.cs @@ -134,6 +134,9 @@ public class PlaylistController(IMediator mediator) : ControllerBase [HttpGet("/api/playlists/{id:int}/items", Name = "GetPlaylistItems")] [Tags("Playlists")] [EndpointSummary("Get the items in a playlist")] + [EndpointDescription( + "Returns the playlist's items and a strong ETag of the playlist'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 PlaylistController(IMediator mediator) : ControllerBase return ApiResults.NotFoundProblem(); } + // The items GET returns children, not the root, so read the playlist's version for the ETag. + ConcurrencyHeaders.SetETag(Response, maybePlaylist.Map(p => p.Version).IfNone(0)); + List items = await mediator.Send(new GetPlaylistItems(id), cancellationToken); return new OkObjectResult(items.Map(ProjectToItemResponse).ToList()); } @@ -168,15 +174,33 @@ public class PlaylistController(IMediator mediator) : ControllerBase [HttpPut("/api/playlists/{id:int}", Name = "UpdatePlaylist")] [Tags("Playlists")] [EndpointSummary("Update a playlist (rename and replace its items)")] + [EndpointDescription( + "Replaces the playlist's name and its full item list. Item indexes are assigned from the array " + + "order. 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(List), 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 Update( int id, [Required] [FromBody] ReplacePlaylistRequest 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 maybePlaylist = await mediator.Send(new GetPlaylistById(id), cancellationToken); if (maybePlaylist.IsNone) { @@ -195,10 +219,25 @@ public class PlaylistController(IMediator mediator) : ControllerBase } Either> result = - await mediator.Send(request.ToCommand(id), cancellationToken); - return result.Match( - Left: error => error.ToErrorResult(), - Right: items => (IActionResult)new OkObjectResult(items.Map(ProjectToItemResponse).ToList())); + await mediator.Send(request.ToCommand(id, ifMatch.ExpectedVersion), cancellationToken); + return await result.Match( + Left: error => Task.FromResult(error.ToErrorResult()), + Right: async _ => + { + // Reload the root (version) FIRST, then the items, so the emitted ETag is never newer than + // the returned items (issue #253 fail-safe ordering; matches BlockController). A None root + // (deleted between commit and reload) is a 404, never a 200 without an ETag. + Option refreshed = + await mediator.Send(new GetPlaylistById(id), cancellationToken); + List items = await mediator.Send(new GetPlaylistItems(id), cancellationToken); + return refreshed.Match( + Some: vm => + { + ConcurrencyHeaders.SetETag(Response, vm.Version); + return (IActionResult)new OkObjectResult(items.Map(ProjectToItemResponse).ToList()); + }, + None: () => ApiResults.NotFoundProblem()); + }); } [HttpDelete("/api/playlists/{id:int}", Name = "DeletePlaylist")] diff --git a/ErsatzTV/Controllers/Api/PlayoutController.cs b/ErsatzTV/Controllers/Api/PlayoutController.cs index c023135d7..42eb40043 100644 --- a/ErsatzTV/Controllers/Api/PlayoutController.cs +++ b/ErsatzTV/Controllers/Api/PlayoutController.cs @@ -295,6 +295,9 @@ public class PlayoutController(IMediator mediator, IEntityLocker entityLocker) : } } + // The GET returns children, so read the playout's version for the concurrency ETag (issue #253). + ConcurrencyHeaders.SetETag(Response, maybePlayout.Map(p => p.Version).IfNone(0)); + List items = await mediator.Send(new GetPlayoutAlternateSchedules(id), cancellationToken); return new OkObjectResult(items.OrderBy(i => i.Index).Select(ToResponse).ToList()); @@ -310,14 +313,22 @@ public class PlayoutController(IMediator mediator, IEntityLocker entityLocker) : "at least one item, and every ProgramScheduleId must exist. Only valid for Classic playouts.")] [EndpointGroupName("general")] [ProducesResponseType(typeof(List), StatusCodes.Status200OK)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status400BadRequest)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status412PreconditionFailed)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task ReplaceAlternateSchedules( int id, [Required] [FromBody] ReplacePlayoutAlternateSchedulesRequest request, CancellationToken cancellationToken) { + IfMatchCondition ifMatch = ConcurrencyHeaders.ParseIfMatch(Request); + if (ifMatch.Kind is IfMatchKind.Malformed) + { + return ConcurrencyHeaders.MalformedIfMatchProblem(); + } + if (entityLocker.IsPlayoutLocked(id)) { return PlayoutLockedProblem(); @@ -358,11 +369,17 @@ public class PlayoutController(IMediator mediator, IEntityLocker entityLocker) : return BaseError.New($"[ProgramScheduleId] {missingScheduleIds[0]} does not exist").ToErrorResult(); } - Either result = await mediator.Send(request.ToCommand(id), cancellationToken); + Either result = + await mediator.Send(request.ToCommand(id, ifMatch.ExpectedVersion), cancellationToken); return await result.Match( Left: error => Task.FromResult(error.ToErrorResult()), Right: async _ => { + // Emit the new ETag (post-bump) so a same-tab second save doesn't 412 against its own write (#253). + Option refreshedPlayout = + await mediator.Send(new GetPlayoutById(id), cancellationToken); + ConcurrencyHeaders.SetETag(Response, refreshedPlayout.Map(p => p.Version).IfNone(0)); + List refreshed = await mediator.Send(new GetPlayoutAlternateSchedules(id), cancellationToken); return (IActionResult)new OkObjectResult(refreshed.OrderBy(i => i.Index).Select(ToResponse).ToList()); @@ -395,6 +412,9 @@ public class PlayoutController(IMediator mediator, IEntityLocker entityLocker) : } } + // The GET returns children, so read the playout's version for the concurrency ETag (issue #253). + ConcurrencyHeaders.SetETag(Response, maybePlayout.Map(p => p.Version).IfNone(0)); + List items = await mediator.Send(new GetPlayoutTemplates(id), cancellationToken); return new OkObjectResult(items.OrderBy(i => i.Index).Select(ToResponse).ToList()); } @@ -408,14 +428,22 @@ public class PlayoutController(IMediator mediator, IEntityLocker entityLocker) : "supplied DecoTemplateId must exist. An empty list clears all templates. Only valid for Block playouts.")] [EndpointGroupName("general")] [ProducesResponseType(typeof(List), StatusCodes.Status200OK)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status400BadRequest)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status412PreconditionFailed)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task ReplaceTemplates( int id, [Required] [FromBody] ReplacePlayoutTemplatesRequest request, CancellationToken cancellationToken) { + IfMatchCondition ifMatch = ConcurrencyHeaders.ParseIfMatch(Request); + if (ifMatch.Kind is IfMatchKind.Malformed) + { + return ConcurrencyHeaders.MalformedIfMatchProblem(); + } + if (entityLocker.IsPlayoutLocked(id)) { return PlayoutLockedProblem(); @@ -463,12 +491,17 @@ public class PlayoutController(IMediator mediator, IEntityLocker entityLocker) : } } - Option result = await mediator.Send(request.ToCommand(id), cancellationToken); + Option result = + await mediator.Send(request.ToCommand(id, ifMatch.ExpectedVersion), cancellationToken); foreach (BaseError error in result) { return error.ToErrorResult(); } + // Emit the new ETag (post-bump) so a same-tab second save doesn't 412 against its own write (#253). + Option refreshedPlayout = await mediator.Send(new GetPlayoutById(id), cancellationToken); + ConcurrencyHeaders.SetETag(Response, refreshedPlayout.Map(p => p.Version).IfNone(0)); + List refreshed = await mediator.Send(new GetPlayoutTemplates(id), cancellationToken); return new OkObjectResult(refreshed.OrderBy(i => i.Index).Select(ToResponse).ToList()); } 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/ReplacePlaylistRequest.cs b/ErsatzTV/Controllers/Api/Requests/ReplacePlaylistRequest.cs index 2e1ad4ef4..6f5bdc5f6 100644 --- a/ErsatzTV/Controllers/Api/Requests/ReplacePlaylistRequest.cs +++ b/ErsatzTV/Controllers/Api/Requests/ReplacePlaylistRequest.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Linq; using ErsatzTV.Application.MediaCollections; using ErsatzTV.Core.Domain; +using LanguageExt; namespace ErsatzTV.Controllers.Api.Requests; @@ -56,8 +57,8 @@ public record PlaylistItemRequest( public record ReplacePlaylistRequest(string? Name, List? Items) { - public ReplacePlaylistItems ToCommand(int id) => - new(id, Name ?? string.Empty, BuildItems()); + public ReplacePlaylistItems ToCommand(int id, Option expectedVersion = default) => + new(id, Name ?? string.Empty, BuildItems(), expectedVersion); // Preview operates on the posted draft, so there is no persisted playlist id (0). public ReplacePlaylistItems ToReplaceCommand() => diff --git a/ErsatzTV/Controllers/Api/Requests/ReplacePlayoutAlternateSchedulesRequest.cs b/ErsatzTV/Controllers/Api/Requests/ReplacePlayoutAlternateSchedulesRequest.cs index b94b3ef89..fac2c6c53 100644 --- a/ErsatzTV/Controllers/Api/Requests/ReplacePlayoutAlternateSchedulesRequest.cs +++ b/ErsatzTV/Controllers/Api/Requests/ReplacePlayoutAlternateSchedulesRequest.cs @@ -8,10 +8,11 @@ public record ReplacePlayoutAlternateSchedulesRequest(List + public ReplacePlayoutAlternateScheduleItems ToCommand(int playoutId, Option expectedVersion = default) => new( playoutId, (Items ?? []) .Select((item, index) => item.ToReplaceItem(index)) - .ToList()); + .ToList(), + expectedVersion); } diff --git a/ErsatzTV/Controllers/Api/Requests/ReplacePlayoutTemplatesRequest.cs b/ErsatzTV/Controllers/Api/Requests/ReplacePlayoutTemplatesRequest.cs index 73bce1d8c..ee0321a74 100644 --- a/ErsatzTV/Controllers/Api/Requests/ReplacePlayoutTemplatesRequest.cs +++ b/ErsatzTV/Controllers/Api/Requests/ReplacePlayoutTemplatesRequest.cs @@ -5,10 +5,11 @@ namespace ErsatzTV.Controllers.Api.Requests; public record ReplacePlayoutTemplatesRequest(List Items) { // Index is assigned from array order (top-to-bottom priority), mirroring the Blazor editor. - public ReplacePlayoutTemplateItems ToCommand(int playoutId) => + public ReplacePlayoutTemplateItems ToCommand(int playoutId, Option expectedVersion = default) => new( playoutId, (Items ?? []) .Select((item, index) => item.ToReplaceItem(index)) - .ToList()); + .ToList(), + expectedVersion); } diff --git a/ErsatzTV/Controllers/Api/Requests/ReplaceScheduleItemsRequest.cs b/ErsatzTV/Controllers/Api/Requests/ReplaceScheduleItemsRequest.cs index 40a8f6864..fd17ec5df 100644 --- a/ErsatzTV/Controllers/Api/Requests/ReplaceScheduleItemsRequest.cs +++ b/ErsatzTV/Controllers/Api/Requests/ReplaceScheduleItemsRequest.cs @@ -4,8 +4,9 @@ namespace ErsatzTV.Controllers.Api.Requests; public record ReplaceScheduleItemsRequest(List Items) { - public ReplaceProgramScheduleItems ToCommand(int scheduleId) => + public ReplaceProgramScheduleItems ToCommand(int scheduleId, Option expectedVersion = default) => new( scheduleId, - (Items ?? []).Select((item, index) => item.ToReplaceCommand(index)).ToList()); + (Items ?? []).Select((item, index) => item.ToReplaceCommand(index)).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/Requests/UpdateCollectionCustomOrderRequest.cs b/ErsatzTV/Controllers/Api/Requests/UpdateCollectionCustomOrderRequest.cs index faaa63714..a8b51e6a8 100644 --- a/ErsatzTV/Controllers/Api/Requests/UpdateCollectionCustomOrderRequest.cs +++ b/ErsatzTV/Controllers/Api/Requests/UpdateCollectionCustomOrderRequest.cs @@ -4,10 +4,11 @@ namespace ErsatzTV.Controllers.Api.Requests; public record UpdateCollectionCustomOrderRequest(List MediaItemIds) { - public UpdateCollectionCustomOrder ToCommand(int collectionId) => + public UpdateCollectionCustomOrder ToCommand(int collectionId, Option expectedVersion = default) => new( collectionId, (MediaItemIds ?? []) .Select((mediaItemId, index) => new MediaItemCustomOrder(mediaItemId, index)) - .ToList()); + .ToList(), + expectedVersion); } diff --git a/ErsatzTV/Controllers/Api/Requests/UpdateMultiCollectionRequest.cs b/ErsatzTV/Controllers/Api/Requests/UpdateMultiCollectionRequest.cs index 07bb99d3c..b5d017df7 100644 --- a/ErsatzTV/Controllers/Api/Requests/UpdateMultiCollectionRequest.cs +++ b/ErsatzTV/Controllers/Api/Requests/UpdateMultiCollectionRequest.cs @@ -6,7 +6,7 @@ namespace ErsatzTV.Controllers.Api.Requests; public record UpdateMultiCollectionRequest(string Name, List Items) { - public UpdateMultiCollection ToCommand(int id) => + public UpdateMultiCollection ToCommand(int id, Option expectedVersion = default) => new( id, Name, @@ -16,5 +16,6 @@ public record UpdateMultiCollectionRequest(string Name, List expectedVersion = default) { (MediaCollectionViewModel collection, MultiCollectionViewModel multiCollection, @@ -28,6 +28,7 @@ public record UpdateRerunCollectionRequest( smartCollection, mediaItem, FirstRunPlaybackOrder, - RerunPlaybackOrder); + RerunPlaybackOrder, + expectedVersion); } } diff --git a/ErsatzTV/Controllers/Api/RerunCollectionController.cs b/ErsatzTV/Controllers/Api/RerunCollectionController.cs index 6faa54031..380f857d1 100644 --- a/ErsatzTV/Controllers/Api/RerunCollectionController.cs +++ b/ErsatzTV/Controllers/Api/RerunCollectionController.cs @@ -48,6 +48,13 @@ public class RerunCollectionController(IMediator mediator) : ControllerBase { Option result = await mediator.Send(new GetRerunCollectionById(id), cancellationToken); + + // This by-id GET is the editor's load endpoint: emit the concurrency ETag (issue #253). + foreach (RerunCollectionViewModel vm in result) + { + ConcurrencyHeaders.SetETag(Response, vm.Version); + } + return result.Map(ProjectToResponseModel).ToGetResult(); } @@ -81,13 +88,21 @@ public class RerunCollectionController(IMediator mediator) : ControllerBase [EndpointSummary("Update a rerun collection")] [EndpointGroupName("general")] [ProducesResponseType(typeof(RerunCollectionResponseModel), 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 Update( int id, [Required] [FromBody] UpdateRerunCollectionRequest request, CancellationToken cancellationToken) { + IfMatchCondition ifMatch = ConcurrencyHeaders.ParseIfMatch(Request); + if (ifMatch.Kind is IfMatchKind.Malformed) + { + return ConcurrencyHeaders.MalformedIfMatchProblem(); + } + if (!RerunCollectionRequestMapping.IsSupportedSelectionType(request.CollectionType)) { return BaseError.New( @@ -95,7 +110,8 @@ public class RerunCollectionController(IMediator mediator) : ControllerBase .ToErrorResult(); } - Either result = await mediator.Send(request.ToCommand(id), cancellationToken); + Either result = + await mediator.Send(request.ToCommand(id, ifMatch.ExpectedVersion), cancellationToken); return await result.Match( Left: error => Task.FromResult(error.ToErrorResult()), Right: async _ => @@ -103,7 +119,12 @@ public class RerunCollectionController(IMediator mediator) : ControllerBase Option rerunCollection = await mediator.Send(new GetRerunCollectionById(id), cancellationToken); return rerunCollection.Match( - Some: vm => (IActionResult)new OkObjectResult(ProjectToResponseModel(vm)), + Some: vm => + { + // Emit the new ETag so a same-tab second save doesn't 412 against its own write (#253). + ConcurrencyHeaders.SetETag(Response, vm.Version); + return (IActionResult)new OkObjectResult(ProjectToResponseModel(vm)); + }, None: () => ApiResults.NotFoundProblem()); }); } diff --git a/ErsatzTV/Controllers/Api/ScheduleController.cs b/ErsatzTV/Controllers/Api/ScheduleController.cs index 0a46e4e16..c325f7824 100644 --- a/ErsatzTV/Controllers/Api/ScheduleController.cs +++ b/ErsatzTV/Controllers/Api/ScheduleController.cs @@ -104,7 +104,9 @@ public class ScheduleController(IMediator mediator) : ControllerBase [EndpointDescription( "Returns the schedule's items plus a computed, best-effort runtime estimate: each item carries a " + "nullable durationEstimate and the envelope carries a nullable totalDurationEstimate. Estimates are " + - "derived from referenced collection/media runtimes and are null when unbounded or unknown.")] + "derived from referenced collection/media runtimes and are null when unbounded or unknown. The " + + "response also carries a strong ETag of the schedule's version; pass that ETag back as If-Match on " + + "the replace (PUT) to detect a concurrent edit (issue #253).")] [EndpointGroupName("general")] [ProducesResponseType(typeof(ScheduleItemsResponseModel), StatusCodes.Status200OK)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] @@ -116,6 +118,9 @@ public class ScheduleController(IMediator mediator) : ControllerBase return ApiResults.NotFoundProblem(); } + // The items GET returns children, not the root, so read the schedule's version for the ETag. + ConcurrencyHeaders.SetETag(Response, schedule.Map(s => s.Version).IfNone(0)); + ProgramScheduleItemsWithDurationViewModel items = await mediator.Send(new GetProgramScheduleItemsWithDurations(id), cancellationToken); return new OkObjectResult(ScheduleItemResponseMapper.ProjectToResponseModel(items)); @@ -143,20 +148,57 @@ public class ScheduleController(IMediator mediator) : ControllerBase [HttpPut("/api/schedules/{id:int}/items")] [Tags("Schedules")] [EndpointSummary("Replace schedule items")] + [EndpointDescription( + "Replaces the schedule's full item list; item indexes are assigned from the array order. 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(List), 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 ReplaceItems( int id, [Required] [FromBody] ReplaceScheduleItemsRequest 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 \"*\"." + }); + } + Either> result = - await mediator.Send(request.ToCommand(id), cancellationToken); - return result - .Map(items => items.Select(ScheduleItemResponseMapper.ProjectToResponseModel).ToList()) - .ToUpdatedResult(); + await mediator.Send(request.ToCommand(id, ifMatch.ExpectedVersion), cancellationToken); + + return await result.Match( + Left: error => Task.FromResult(error.ToErrorResult()), + Right: async _ => + { + // Reload the root (version) FIRST, then the items, so the emitted ETag is never newer than + // the returned items (issue #253 fail-safe ordering; matches BlockController). A None root + // (deleted between commit and reload) is a 404, never a 200 without an ETag. + Option refreshed = + await mediator.Send(new GetProgramScheduleById(id), cancellationToken); + List items = + await mediator.Send(new GetProgramScheduleItems(id), cancellationToken); + return refreshed.Match( + 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( + items.Select(ScheduleItemResponseMapper.ProjectToResponseModel).ToList()); + }, + None: () => ApiResults.NotFoundProblem()); + }); } [HttpDelete("/api/schedules/{id:int}/items/{itemId:int}")] diff --git a/ErsatzTV/Controllers/Api/TemplateController.cs b/ErsatzTV/Controllers/Api/TemplateController.cs index e02f87209..5bf3a11f8 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,25 @@ 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 _ => { + // Reload the root (version) FIRST, then the items, so the emitted ETag is never newer than + // the returned items (issue #253 fail-safe ordering; matches BlockController). Returning the + // handler's item snapshot alongside a separately re-queried version could pair stale items + // with a newer ETag — a client would then silently overwrite the interleaving write. 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/ErsatzTV/Extensions/ConcurrencyHeaders.cs b/ErsatzTV/Extensions/ConcurrencyHeaders.cs index c3dafae8d..3735cb120 100644 --- a/ErsatzTV/Extensions/ConcurrencyHeaders.cs +++ b/ErsatzTV/Extensions/ConcurrencyHeaders.cs @@ -1,6 +1,7 @@ using System.Globalization; using LanguageExt; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Primitives; namespace ErsatzTV.Extensions; @@ -74,4 +75,18 @@ public static class ConcurrencyHeaders public static void SetETag(HttpResponse response, int version) => response.Headers.ETag = $"\"{version}\""; + + /// + /// The 400 response for a syntactically-invalid If-Match header + /// (). Shared by the replace-all PUT endpoints so the + /// fail-safe rejection wording stays consistent (issue #253 §7a). + /// + public static IActionResult MalformedIfMatchProblem() => + 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 \"*\"." + }); } diff --git a/ErsatzTV/wwwroot/openapi/v1.json b/ErsatzTV/wwwroot/openapi/v1.json index 800009364..89b9f5d21 100644 --- a/ErsatzTV/wwwroot/openapi/v1.json +++ b/ErsatzTV/wwwroot/openapi/v1.json @@ -2981,6 +2981,26 @@ "204": { "description": "No Content" }, + "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": { @@ -3001,6 +3021,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": { @@ -4039,7 +4079,7 @@ "DecoTemplates" ], "summary": "Replace a deco template and its items", - "description": "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.", + "description": "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. 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", @@ -4097,6 +4137,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": { @@ -4117,6 +4177,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": { @@ -4146,6 +4226,7 @@ "DecoTemplates" ], "summary": "Get deco template items", + "description": "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).", "parameters": [ { "name": "id", @@ -8321,6 +8402,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": { @@ -8341,6 +8442,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": { @@ -8901,6 +9022,7 @@ "Playlists" ], "summary": "Update a playlist (rename and replace its items)", + "description": "Replaces the playlist's name and its full item list. Item indexes are assigned from the array order. 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.", "operationId": "UpdatePlaylist", "parameters": [ { @@ -8968,6 +9090,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": { @@ -8988,6 +9130,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": { @@ -9080,6 +9242,7 @@ "Playlists" ], "summary": "Get the items in a playlist", + "description": "Returns the playlist's items and a strong ETag of the playlist's version. Pass that ETag back as If-Match on the replace (PUT) to detect a concurrent edit (issue #253).", "operationId": "GetPlaylistItems", "parameters": [ { @@ -10153,6 +10316,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": { @@ -10193,6 +10376,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": { @@ -10379,6 +10582,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": { @@ -10419,6 +10642,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": { @@ -11879,6 +12122,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": { @@ -11899,6 +12162,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": { @@ -12609,7 +12892,7 @@ "Schedules" ], "summary": "Get schedule items", - "description": "Returns the schedule's items plus a computed, best-effort runtime estimate: each item carries a nullable durationEstimate and the envelope carries a nullable totalDurationEstimate. Estimates are derived from referenced collection/media runtimes and are null when unbounded or unknown.", + "description": "Returns the schedule's items plus a computed, best-effort runtime estimate: each item carries a nullable durationEstimate and the envelope carries a nullable totalDurationEstimate. Estimates are derived from referenced collection/media runtimes and are null when unbounded or unknown. The response also carries a strong ETag of the schedule's version; pass that ETag back as If-Match on the replace (PUT) to detect a concurrent edit (issue #253).", "parameters": [ { "name": "id", @@ -12773,6 +13056,7 @@ "Schedules" ], "summary": "Replace schedule items", + "description": "Replaces the schedule's full item list; item indexes are assigned from the array order. 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", @@ -12839,6 +13123,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": { @@ -12859,6 +13163,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": { @@ -15244,7 +15568,7 @@ "Templates" ], "summary": "Replace a template and its items", - "description": "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).", + "description": "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). 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", @@ -15302,6 +15626,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": { @@ -15322,6 +15666,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": { @@ -15351,6 +15715,7 @@ "Templates" ], "summary": "Get template items", + "description": "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).", "parameters": [ { "name": "id", @@ -21423,6 +21788,11 @@ "useCustomPlaybackOrder": { "type": "boolean" }, + "version": { + "type": "integer", + "format": "int32", + "default": 0 + }, "mediaItemId": { "type": "integer", "format": "int32" @@ -23309,7 +23679,8 @@ "treatCollectionsAsShows", "shuffleScheduleItems", "randomStartPoint", - "fixedStartTimeBehavior" + "fixedStartTimeBehavior", + "version" ], "type": "object", "properties": { @@ -23337,6 +23708,10 @@ }, "fixedStartTimeBehavior": { "$ref": "#/components/schemas/FixedStartTimeBehavior" + }, + "version": { + "type": "integer", + "format": "int32" } } }, diff --git a/docs/api-conventions.md b/docs/api-conventions.md index 9e3deed7e..3ed6b2514 100644 --- a/docs/api-conventions.md +++ b/docs/api-conventions.md @@ -311,8 +311,21 @@ GET handlers that feed an ordered list must also `.OrderBy(i => i.Index)` — id 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 2–4 fan the same -recipe across the other aggregates. +fresher edit (issue #253). The Block endpoints are the reference implementation; **PR2 fanned the same +recipe onto Template, DecoTemplate, Playlist, and schedule-items** (`PUT /api/templates/{id}`, +`/api/deco-templates/{id}`, `/api/playlists/{id}`, `/api/schedules/{id}/items`); PR3 covers the +Diff/Scalar aggregates (Collection, Playout ×2, MultiCollection, RerunCollection) and PR4 is the Phase-2 +428 flip. + +Each replace PUT keeps its **own** existing 200 body shape (Template/DecoTemplate return a +`…WithItemsResponseModel`, Block likewise; Playlist and schedule-items return the item array) and adds the +ETag as a **header only** — except schedules: `ScheduleController` returns the Application-layer +`ProgramScheduleViewModel` *directly* (no `ProgramScheduleResponseModel` exists), so the added +`int Version` also surfaces as a redundant `version` field in the `GET /api/schedules[/{id}]` bodies. That +is intentional and harmless (the ETag remains the authority); introducing a ResponseModel purely to hide +one field was judged disproportionate. Every replace PUT's *sibling config writers* bump `Version` too +(Playlist: the five `Add*ToPlaylist` handlers; schedule: `AddProgramScheduleItem` / `DeleteProgramScheduleItem` +/ `UpdateProgramSchedule`; Template/DecoTemplate have none). **Token.** Each versioned root implements `IVersionedAggregate` (`int Version`, EF-mapped with `.IsConcurrencyToken()` in its `IEntityTypeConfiguration`). A single dual-provider migration @@ -361,6 +374,24 @@ save → 412 (prove it non-vacuous by dropping `.IsConcurrencyToken()` and watch Phase 2 (a later PR) flips a missing `If-Match` from force-write to **428 Precondition Required** once every editor echoes and one release soaks. +**Fan-out status.** Block (#2) is the reference. **PR2** wired the RR + Reconcile aggregates +(Template #3, DecoTemplate #4, Playlist #5, schedule-items #1). **PR3** wired the Diff + Scalar aggregates: +Collection custom-order #6, Playout alternate-schedules #7 and templates #8 (both share `Playout.Version`; +their `catch(Exception)`→422 handlers were restructured so the guard's `PreconditionFailedError` Left +returns before the catch — §9/H1), MultiCollection #9, and RerunCollection #10. Two M2 gate notes for the +`SaveChangesAsync() > 0` handlers: the RerunCollection/Collection-custom-order refresh now runs on any +successful save (the bump makes the gate always-true); MultiCollection keeps its "name-only change → no +playout rebuild" optimization by bumping on the **first** (name) save so the **second** (items) save's +`> 0` still means "items changed". The one bulk writer in scope, `UpdateDefaultDecoHandler`, bumps via +`.SetProperty(x => x.Version, x => x.Version + 1)` (bulk `ExecuteUpdate` can't throw the concurrency +exception, so it needs no guard). **Deferred (tracked follow-up):** the *other* same-root config +writers — non-bulk siblings like `UpdateCollectionHandler`/`RemoveItemsFromCollectionHandler`, +`UpdatePlayoutHandler` and the `ScheduleFile` handlers, and the repository-mediated `Add*ToCollection` +family — do **not** yet rotate their aggregate's ETag. The primary endpoints' own bump+guard fully cover +the two-tab lost-update this contract targets; the deferred writers only affect cross-editor ETag rotation, +and adding an unconditional bump to a handler that uses plain `SaveChangesAsync` (not the guard) would open +a new `DbUpdateConcurrencyException`→500 path — so they need a uniform guard+bump pass of their own. + ## 7b. Post-commit side effects run on `CancellationToken.None` Once a command handler's `await dbContext.SaveChangesAsync(cancellationToken)` (or repository upsert) diff --git a/docs/ci-cd.md b/docs/ci-cd.md index 51b28786e..02bb517b4 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -242,6 +242,55 @@ outside a transaction and warn at startup; they can't be rolled back mid-migrati migrations carefully (this is part of what motivated the apply-to-fresh check before the prod cutover, server-management#481). +## Pre-commit hooks (web/) + +The repo uses **husky** git hooks (installed via `web/`'s **lint-staged** + npm) to catch +lint/format/type/API-drift errors locally, before they reach CI. Because the git root and +the npm project dir differ (monorepo: no root `package.json`, the JS/TS project lives +entirely in `web/`), the wiring is: + +- `husky` + `lint-staged` are devDependencies of `web/package.json` (not a root package — + there isn't one). +- The committed hook scripts live at the repo root: `.husky/pre-commit`, `.husky/pre-push`, + `.husky/commit-msg`. +- `web/package.json`'s `prepare` script (`cd .. && husky`) runs on `npm install` inside + `web/` and points git at the repo-root `.husky` dir (`git config core.hooksPath + .husky/_` — the `_` subdir is husky's generated internal dir, gitignored via its own + `.husky/_/.gitignore`; only the hook scripts themselves are committed). This works + because npm keeps `web/node_modules/.bin` on `PATH` for the `prepare` script even after + it `cd ..`s to the repo root (which husky's init requires — it hard-checks for `.git` + in the *current* directory). + +**The four hooks:** + +1. **`pre-commit`** — (a) `cd web && npx lint-staged`: runs `eslint --fix` on staged + `web/src/**/*.{ts,tsx}` files, then a project-wide `npm run typecheck` (`tsc -b` isn't + file-scoped, so it runs the full check, but only when a `.ts`/`.tsx` file is staged); + (b) back at the repo root, if any **`*.cs`** files are staged, `dotnet format + ErsatzTV.sln --verify-no-changes --include ` — a formatting violation + blocks the commit. The .cs step is **skipped entirely when no .cs is staged**, so + web-only commits don't pay the sln-load cost; when it does run it's scoped to the staged + files (~6-7s wall in practice, dominated by the workspace load). +2. **`pre-push`** — CI-parity gate: `cd web && npm run check:api && npm run lint && npm run + typecheck && npm run build`. `check:api` guards generated-OpenAPI drift + (`ErsatzTV/wwwroot/openapi/v1.json` → `web/src/api/generated/v1.d.ts`); the full + lint/typecheck/build catch a staged change that breaks an *unstaged* file (lint-staged + only sees staged files). Any failure blocks the push. +3. **`commit-msg`** — enforces the CLAUDE.md protocol: the message must carry a + `Co-Authored-By:` trailer, else the commit is rejected. Merge commits are exempt + (detected via `git rev-parse --verify MERGE_HEAD`). + +- **Worktree/subdir gotcha**: git exports `GIT_DIR` (and friends) while running hooks. In a + worktree or any subdir, an explicit `GIT_DIR` makes nested `git` commands mislocate the + working tree — `pre-push`'s `check:api` (`git diff --exit-code`, run from `web/`) then + silently reports "no diff" and lets drift through. `pre-push` therefore `unset`s + `GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE` first. (`pre-commit`'s `.cs` collection uses + `git diff --cached`, index-vs-HEAD, which needs only `GIT_DIR` and is unaffected.) +- **Practical effect**: a fresh `web/` `npm install` (after cloning or pulling this change) + installs all four hooks automatically — no separate setup step. Commits that touch only + non-`web/`, non-`.cs` files skip linting/formatting (lint-staged no-ops with nothing to + run, the `.cs` step is skipped). + ## Registry Gitea Packages, HTTP-only at `192.168.1.95:3000`. the `ci-runner` VM's Docker daemon (192.168.1.127) has it as an diff --git a/docs/decisions.md b/docs/decisions.md index 8cfcb2f13..d9f37517d 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -778,6 +778,48 @@ the versioned aggregates — several of which this sweep also touched (post-comm region). Low git-conflict risk, but merge `main` in and expect to see the `CancellationToken.None` convention already present on the post-commit enqueues. +## 2026-07-11 — #253 PR3: Diff + Scalar concurrency fan-out (Collection / Playout×2 / MultiCollection / RerunCollection) + +**Context.** PR3 of the #253 optimistic-concurrency arc fans the frozen Block recipe (api-conventions §7a) +across the five Diff/Scalar aggregates. Three judgment calls beyond the mechanical copy: + +**H1 — Playout `catch(Exception)`→422.** The two Playout replace handlers wrap `SaveChangesAsync` in a +`catch(Exception)` that maps any exception to a bare `BaseError` (→ 422). Rather than let the guard's +concurrency failure be reshaped into a 422, the guarded save (`SaveChangesWithConcurrencyGuard`) returns a +`PreconditionFailedError` **Left as a value** and the handler returns it before the post-commit block — +so it never reaches the catch. Proven by the pre-check-subtype tests (a `.Apply` flatten would fail +`ShouldBeOfType`) plus a non-vacuous Playout racing-save test. + +**M2 — the `SaveChangesAsync() > 0` gates.** RerunCollection and Collection-custom-order run their +playout-refresh **unconditionally** on a successful save (the unconditional `Version++` makes the old gate +always-true; the "nothing changed" branch is dead). MultiCollection is the exception: it saved the name +first specifically so a name-only change wouldn't rebuild playouts, so we bump `Version` on that **first** +save and leave the **second** (items) save's `> 0` gate intact — a name-only edit still bumps + rotates the +ETag but does not rebuild. Enumerating every behavior the gate provided before reworking it (the #232 lesson). + +**Sibling-writer scope (deferred).** §7a's config-only boundary says every writer of an aggregate's +editor-visible config bumps `Version`. PR3 ships the five primary endpoints' full contract + the one +design-named bulk writer (`UpdateDefaultDecoHandler`, safe via `.SetProperty`). It **defers** the other +same-root non-bulk config writers (`UpdateCollectionHandler`, `RemoveItemsFromCollectionHandler`, +`UpdatePlayoutHandler`, the `ScheduleFile` handlers) and the repository-mediated `Add*ToCollection` family. +Rationale: the primary endpoints' own bump+guard fully cover the two-tab lost-update the issue targets; +the deferred writers only affect cross-editor ETag *rotation*, and adding an unconditional bump to a handler +that uses plain `SaveChangesAsync` (not the guard) converts a latent lost-update into a **new 500** +(`DbUpdateConcurrencyException`) — doing it safely needs a uniform guard+bump+412 pass of its own, better +done with the #197 contract work. Tracked as a follow-up issue. + +**VMs.** `Playout.Version` surfaces via `PlayoutNameViewModel` (required arg); the three collection VMs +(`MediaCollectionViewModel`, `MultiCollectionViewModel`, `RerunCollectionViewModel`) carry `int Version = 0` +(defaulted — 0 for the selection-placeholder constructions, real value from the Mapper projection). +Header-only via ETag, never echoed in a response body (the Block precedent). + +**Post-merge addendum (PR3 review, #269).** Activating the `Version` token means EF guards *every* root +UPDATE, so non-participating root-scalar writers that use plain `SaveChangesAsync` (playout settings / +schedule-file / on-demand-checkpoint, collection name) would 500 on a concurrent bump. The realistic +UPDATE writers were fixed in-PR with `ConcurrencyExtensions.SaveChangesForcingVersion` (Phase-1 +force-write on conflict: adopt the stored token, retry, never revert the concurrent bump). The deferral +above is re-scoped to the DELETE handlers + repository `Add*` writers only (→ #269). + ## 2026-07-11 — External Collections in the SPA derives from `getMediaSources()`; collections scans are optimistically bounded (#91b Libraries.razor parity) **Context.** The last SPA pre-work before deleting Blazor `Libraries.razor` was wiring the shipped diff --git a/docs/handoffs/chicorytv-issue-queue.md b/docs/handoffs/chicorytv-issue-queue.md index 409c11258..3c66cd7e0 100644 --- a/docs/handoffs/chicorytv-issue-queue.md +++ b/docs/handoffs/chicorytv-issue-queue.md @@ -229,3 +229,17 @@ HARD CONSTRAINTS: media source (fixed loop order), prescribing the shipped library-scan §C7 pattern instead (lock once per provider, batch-enqueue, release on the `Unlock: isLast` message). Escalate the FIX approach too, not just the finding. +- **Activating an `IsConcurrencyToken` exposes EVERY writer of that root, not just the opt-in ones** + (2026-07-11, #253 PR3, review-caught HIGH): once ANY handler bumps a root's version (making the token + live), EF appends `WHERE Version=@orig` to *every* UPDATE/DELETE of that row — so a *separate* + root-scalar writer that saves via plain `SaveChangesAsync` (and doesn't even bump) now throws + `DbUpdateConcurrencyException`→500 on a concurrent bump. Block (PR1) had a single root writer → no + exposure, which is why PR1/PR2 reviews didn't flag it; Playout/Collection (PR3) have several + (settings/schedule-file/checkpoint/name vs. the bumping alt-sched/template/deco/custom-order) → real + two-tab 500. Fix pattern: `ConcurrencyExtensions.SaveChangesForcingVersion` — on conflict, adopt the + stored token as BOTH `OriginalValue` and `CurrentValue` on each `IVersionedAggregate` entry and retry + (client-wins merge scoped to the token; Phase-1 force-write for a missing If-Match; never reverts the + concurrent bump). When you activate a token on aggregate X, grep for ALL other root-scalar UPDATE/DELETE + writers of X (not just the editor) and force-write or guard them. **Suspected open case: PR2's + `ProgramSchedule`** (`UpdateProgramScheduleHandler` vs. the bumping `ReplaceProgramScheduleItems`) — audit + for #197/#269/PR4. diff --git a/docs/spa-conventions.md b/docs/spa-conventions.md index e272858da..50de7523e 100644 --- a/docs/spa-conventions.md +++ b/docs/spa-conventions.md @@ -153,6 +153,12 @@ fresher edit: "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`. +- **List-derived editors** (Multi/Rerun collections open the editor from a paged-list row, not a per-record + GET): fetch the single record via the `…WithMeta` by-id helper (`getMultiCollectionWithMeta` / + `getRerunCollectionWithMeta`) when the editor mounts, and build the draft from **that** response — so the + ETag and the draft data come from one read and stay consistent (the fail-safe ordering; don't pair a + list-row draft with a separately-fetched ETag). Editors that navigate back to the list on save (Playout, + Multi/Rerun, Collection reorder) need only the 412 branch — no post-save ETag rotation. ## 5. Artwork rendering diff --git a/web/package-lock.json b/web/package-lock.json index a183b729a..2bfc02cb3 100644 --- a/web/package-lock.json +++ b/web/package-lock.json @@ -27,7 +27,9 @@ "eslint-plugin-react-hooks": "7.1.1", "eslint-plugin-react-refresh": "0.5.3", "globals": "17.7.0", + "husky": "^9.1.7", "jsdom": "29.1.1", + "lint-staged": "^17.0.8", "typescript-eslint": "8.62.1", "vitest": "4.1.9" } @@ -1670,6 +1672,22 @@ "url": "https://github.com/sponsors/epoberezkin" } }, + "node_modules/ansi-escapes": { + "version": "7.3.0", + "resolved": "https://registry.npmjs.org/ansi-escapes/-/ansi-escapes-7.3.0.tgz", + "integrity": "sha512-BvU8nYgGQBxcmMuEeUEmNTvrMVjJNSH7RgW24vXexN4Ven6qCvy4TntnvlnwnMLTVlcRQQdbRY8NKnaIoeWDNg==", + "dev": true, + "license": "MIT", + "dependencies": { + "environment": "^1.0.0" + }, + "engines": { + "node": ">=18" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/ansi-regex": { "version": "5.0.1", "resolved": "https://registry.npmjs.org/ansi-regex/-/ansi-regex-5.0.1.tgz", @@ -1826,6 +1844,39 @@ "node": ">=18" } }, + "node_modules/cli-cursor": { + "version": "5.0.0", + "resolved": "https://registry.npmjs.org/cli-cursor/-/cli-cursor-5.0.0.tgz", + "integrity": "sha512-aCj4O5wKyszjMmDT4tZj93kxyydN/K5zPWSCe6/0AV/AA1pqe5ZBIw0a2ZfPQV7lL5/yb5HsUreJ6UFAF1tEQw==", + "dev": true, + "license": "MIT", + "dependencies": { + "restore-cursor": "^5.0.0" + }, + "engines": { + "node": ">=18" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, + "node_modules/cli-truncate": { + "version": "5.2.0", + "resolved": "https://registry.npmjs.org/cli-truncate/-/cli-truncate-5.2.0.tgz", + "integrity": "sha512-xRwvIOMGrfOAnM1JYtqQImuaNtDEv9v6oIYAs4LIHwTiKee8uwvIi363igssOC0O5U04i4AlENs79LQLu9tEMw==", + "dev": true, + "license": "MIT", + "dependencies": { + "slice-ansi": "^8.0.0", + "string-width": "^8.2.0" + }, + "engines": { + "node": ">=20" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/convert-source-map": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/convert-source-map/-/convert-source-map-2.0.0.tgz", @@ -1956,6 +2007,13 @@ "dev": true, "license": "ISC" }, + "node_modules/emoji-regex": { + "version": "10.6.0", + "resolved": "https://registry.npmjs.org/emoji-regex/-/emoji-regex-10.6.0.tgz", + "integrity": "sha512-toUI84YS5YmxW219erniWD0CIVOo46xGKColeNQRgOzDorgBi1v4D71/OFzgD9GO2UGKIv1C3Sp8DAn0+j5w7A==", + "dev": true, + "license": "MIT" + }, "node_modules/entities": { "version": "8.0.0", "resolved": "https://registry.npmjs.org/entities/-/entities-8.0.0.tgz", @@ -1969,6 +2027,19 @@ "url": "https://github.com/fb55/entities?sponsor=1" } }, + "node_modules/environment": { + "version": "1.1.0", + "resolved": "https://registry.npmjs.org/environment/-/environment-1.1.0.tgz", + "integrity": "sha512-xUtoPkMggbz0MPyPiIWr1Kp4aeWJjDZ6SMvURhimjdZgsRuDplF5/s9hcgGhyXMhs+6vpnuoiZ2kFiu3FMnS8Q==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=18" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/es-module-lexer": { "version": "2.3.0", "resolved": "https://registry.npmjs.org/es-module-lexer/-/es-module-lexer-2.3.0.tgz", @@ -2194,6 +2265,13 @@ "node": ">=0.10.0" } }, + "node_modules/eventemitter3": { + "version": "5.0.4", + "resolved": "https://registry.npmjs.org/eventemitter3/-/eventemitter3-5.0.4.tgz", + "integrity": "sha512-mlsTRyGaPBjPedk6Bvw+aqbsXDtoAyAzm5MO7JgU+yVRyMQ5O8bD4Kcci7BS85f93veegeCPkL8R4GLClnjLFw==", + "dev": true, + "license": "MIT" + }, "node_modules/expect-type": { "version": "1.4.0", "resolved": "https://registry.npmjs.org/expect-type/-/expect-type-1.4.0.tgz", @@ -2317,6 +2395,19 @@ "node": ">=6.9.0" } }, + "node_modules/get-east-asian-width": { + "version": "1.6.0", + "resolved": "https://registry.npmjs.org/get-east-asian-width/-/get-east-asian-width-1.6.0.tgz", + "integrity": "sha512-QRbvDIbx6YklUe6RxeTeleMR0yv3cYH6PsPZHcnVn7xv7zO1BHN8r0XETu8n6Ye3Q+ahtSarc3WgtNWmehIBfA==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=18" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/glob-parent": { "version": "6.0.2", "resolved": "https://registry.npmjs.org/glob-parent/-/glob-parent-6.0.2.tgz", @@ -2379,6 +2470,22 @@ "node": "^20.19.0 || ^22.12.0 || >=24.0.0" } }, + "node_modules/husky": { + "version": "9.1.7", + "resolved": "https://registry.npmjs.org/husky/-/husky-9.1.7.tgz", + "integrity": "sha512-5gs5ytaNjBrh5Ow3zrvdUUY+0VxIuWVL4i9irt6friV+BqdCfmV11CQTWMiBYWHbXhco+J1kHfTOUkePhCDvMA==", + "dev": true, + "license": "MIT", + "bin": { + "husky": "bin.js" + }, + "engines": { + "node": ">=18" + }, + "funding": { + "url": "https://github.com/sponsors/typicode" + } + }, "node_modules/ignore": { "version": "5.3.2", "resolved": "https://registry.npmjs.org/ignore/-/ignore-5.3.2.tgz", @@ -2419,6 +2526,22 @@ "node": ">=0.10.0" } }, + "node_modules/is-fullwidth-code-point": { + "version": "5.1.0", + "resolved": "https://registry.npmjs.org/is-fullwidth-code-point/-/is-fullwidth-code-point-5.1.0.tgz", + "integrity": "sha512-5XHYaSyiqADb4RnZ1Bdad6cPp8Toise4TzEjcOYDHZkTCbKgiUl7WTUCpNWHuxmDt91wnsZBc9xinNzopv3JMQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "get-east-asian-width": "^1.3.1" + }, + "engines": { + "node": ">=18" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/is-glob": { "version": "4.0.3", "resolved": "https://registry.npmjs.org/is-glob/-/is-glob-4.0.3.tgz", @@ -2836,6 +2959,48 @@ "url": "https://opencollective.com/parcel" } }, + "node_modules/lint-staged": { + "version": "17.0.8", + "resolved": "https://registry.npmjs.org/lint-staged/-/lint-staged-17.0.8.tgz", + "integrity": "sha512-B2P/d+jVW0UXOQ0MVMLrB/9ydA1P+zz6jYfdrbbEd9ur3S2rcbduFWKiUCC02Sm5hbC8nrm7y24WuYMG54HfxA==", + "dev": true, + "license": "MIT", + "dependencies": { + "listr2": "^10.2.1", + "picomatch": "^4.0.4", + "string-argv": "^0.3.2", + "tinyexec": "^1.2.4" + }, + "bin": { + "lint-staged": "bin/lint-staged.js" + }, + "engines": { + "node": ">=22.22.1" + }, + "funding": { + "url": "https://opencollective.com/lint-staged" + }, + "optionalDependencies": { + "yaml": "^2.9.0" + } + }, + "node_modules/listr2": { + "version": "10.2.2", + "resolved": "https://registry.npmjs.org/listr2/-/listr2-10.2.2.tgz", + "integrity": "sha512-JtNtbZj8q5BnDMR7trpwvwk3RIrANtIVzEUm8w7amp6xelLgyuq+4WZoTH913XaQAoH/cNdYhaNzBPA2U3xbDw==", + "dev": true, + "license": "MIT", + "dependencies": { + "cli-truncate": "^5.2.0", + "eventemitter3": "^5.0.4", + "log-update": "^6.1.0", + "rfdc": "^1.4.1", + "wrap-ansi": "^10.0.0" + }, + "engines": { + "node": ">=22.13.0" + } + }, "node_modules/locate-path": { "version": "6.0.0", "resolved": "https://registry.npmjs.org/locate-path/-/locate-path-6.0.0.tgz", @@ -2852,6 +3017,92 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/log-update": { + "version": "6.1.0", + "resolved": "https://registry.npmjs.org/log-update/-/log-update-6.1.0.tgz", + "integrity": "sha512-9ie8ItPR6tjY5uYJh8K/Zrv/RMZ5VOlOWvtZdEHYSTFKZfIBPQa9tOAEeAWhd+AnIneLJ22w5fjOYtoutpWq5w==", + "dev": true, + "license": "MIT", + "dependencies": { + "ansi-escapes": "^7.0.0", + "cli-cursor": "^5.0.0", + "slice-ansi": "^7.1.0", + "strip-ansi": "^7.1.0", + "wrap-ansi": "^9.0.0" + }, + "engines": { + "node": ">=18" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, + "node_modules/log-update/node_modules/ansi-styles": { + "version": "6.2.3", + "resolved": "https://registry.npmjs.org/ansi-styles/-/ansi-styles-6.2.3.tgz", + "integrity": "sha512-4Dj6M28JB+oAH8kFkTLUo+a2jwOFkuqb3yucU0CANcRRUbxS0cP0nZYCGjcc3BNXwRIsUVmDGgzawme7zvJHvg==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=12" + }, + "funding": { + "url": "https://github.com/chalk/ansi-styles?sponsor=1" + } + }, + "node_modules/log-update/node_modules/slice-ansi": { + "version": "7.1.2", + "resolved": "https://registry.npmjs.org/slice-ansi/-/slice-ansi-7.1.2.tgz", + "integrity": "sha512-iOBWFgUX7caIZiuutICxVgX1SdxwAVFFKwt1EvMYYec/NWO5meOJ6K5uQxhrYBdQJne4KxiqZc+KptFOWFSI9w==", + "dev": true, + "license": "MIT", + "dependencies": { + "ansi-styles": "^6.2.1", + "is-fullwidth-code-point": "^5.0.0" + }, + "engines": { + "node": ">=18" + }, + "funding": { + "url": "https://github.com/chalk/slice-ansi?sponsor=1" + } + }, + "node_modules/log-update/node_modules/string-width": { + "version": "7.2.0", + "resolved": "https://registry.npmjs.org/string-width/-/string-width-7.2.0.tgz", + "integrity": "sha512-tsaTIkKW9b4N+AEj+SVA+WhJzV7/zMhcSu78mLKWSk7cXMOSHsBKFWUs0fWwq8QyK3MgJBQRX6Gbi4kYbdvGkQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "emoji-regex": "^10.3.0", + "get-east-asian-width": "^1.0.0", + "strip-ansi": "^7.1.0" + }, + "engines": { + "node": ">=18" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, + "node_modules/log-update/node_modules/wrap-ansi": { + "version": "9.0.2", + "resolved": "https://registry.npmjs.org/wrap-ansi/-/wrap-ansi-9.0.2.tgz", + "integrity": "sha512-42AtmgqjV+X1VpdOfyTGOYRi0/zsoLqtXQckTmqTeybT+BDIbM/Guxo7x3pE2vtpr1ok6xRqM9OpBe+Jyoqyww==", + "dev": true, + "license": "MIT", + "dependencies": { + "ansi-styles": "^6.2.1", + "string-width": "^7.0.0", + "strip-ansi": "^7.1.0" + }, + "engines": { + "node": ">=18" + }, + "funding": { + "url": "https://github.com/chalk/wrap-ansi?sponsor=1" + } + }, "node_modules/lru-cache": { "version": "5.1.1", "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-5.1.1.tgz", @@ -2899,6 +3150,19 @@ "dev": true, "license": "CC0-1.0" }, + "node_modules/mimic-function": { + "version": "5.0.1", + "resolved": "https://registry.npmjs.org/mimic-function/-/mimic-function-5.0.1.tgz", + "integrity": "sha512-VP79XUPxV2CigYP3jWwAUFSku2aKqBH7uTAapFWCBqutsbmDo96KY5o8uh6U+/YSIn5OxJnXp73beVkpqMIGhA==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=18" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/min-indent": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/min-indent/-/min-indent-1.0.1.tgz", @@ -2981,6 +3245,22 @@ "node": ">=12.20.0" } }, + "node_modules/onetime": { + "version": "7.0.0", + "resolved": "https://registry.npmjs.org/onetime/-/onetime-7.0.0.tgz", + "integrity": "sha512-VXJjc87FScF88uafS3JllDgvAm+c/Slfz06lorj2uAY34rlUu0Nt+v8wreiImcrgAjjIHp1rXpTDlLOGw29WwQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "mimic-function": "^5.0.0" + }, + "engines": { + "node": ">=18" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/optionator": { "version": "0.9.4", "resolved": "https://registry.npmjs.org/optionator/-/optionator-0.9.4.tgz", @@ -3206,6 +3486,30 @@ "node": ">=0.10.0" } }, + "node_modules/restore-cursor": { + "version": "5.1.0", + "resolved": "https://registry.npmjs.org/restore-cursor/-/restore-cursor-5.1.0.tgz", + "integrity": "sha512-oMA2dcrw6u0YfxJQXm342bFKX/E4sG9rbTzO9ptUcR/e8A33cHuvStiYOwH7fszkZlZ1z/ta9AAoPk2F4qIOHA==", + "dev": true, + "license": "MIT", + "dependencies": { + "onetime": "^7.0.0", + "signal-exit": "^4.1.0" + }, + "engines": { + "node": ">=18" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, + "node_modules/rfdc": { + "version": "1.4.1", + "resolved": "https://registry.npmjs.org/rfdc/-/rfdc-1.4.1.tgz", + "integrity": "sha512-q1b3N5QkRUWUl7iyylaaj3kOpIT0N2i9MqIEQXP73GVsN9cw3fdx8X63cEmWhJGi2PPCF23Ijp7ktmd39rawIA==", + "dev": true, + "license": "MIT" + }, "node_modules/rolldown": { "version": "1.1.4", "resolved": "https://registry.npmjs.org/rolldown/-/rolldown-1.1.4.tgz", @@ -3298,6 +3602,49 @@ "dev": true, "license": "ISC" }, + "node_modules/signal-exit": { + "version": "4.1.0", + "resolved": "https://registry.npmjs.org/signal-exit/-/signal-exit-4.1.0.tgz", + "integrity": "sha512-bzyZ1e88w9O1iNJbKnOlvYTrWPDl46O1bG0D3XInv+9tkPrxrN8jUUTiFlDkkmKWgn1M6CfIA13SuGqOa9Korw==", + "dev": true, + "license": "ISC", + "engines": { + "node": ">=14" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, + "node_modules/slice-ansi": { + "version": "8.0.0", + "resolved": "https://registry.npmjs.org/slice-ansi/-/slice-ansi-8.0.0.tgz", + "integrity": "sha512-stxByr12oeeOyY2BlviTNQlYV5xOj47GirPr4yA1hE9JCtxfQN0+tVbkxwCtYDQWhEKWFHsEK48ORg5jrouCAg==", + "dev": true, + "license": "MIT", + "dependencies": { + "ansi-styles": "^6.2.3", + "is-fullwidth-code-point": "^5.1.0" + }, + "engines": { + "node": ">=20" + }, + "funding": { + "url": "https://github.com/chalk/slice-ansi?sponsor=1" + } + }, + "node_modules/slice-ansi/node_modules/ansi-styles": { + "version": "6.2.3", + "resolved": "https://registry.npmjs.org/ansi-styles/-/ansi-styles-6.2.3.tgz", + "integrity": "sha512-4Dj6M28JB+oAH8kFkTLUo+a2jwOFkuqb3yucU0CANcRRUbxS0cP0nZYCGjcc3BNXwRIsUVmDGgzawme7zvJHvg==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=12" + }, + "funding": { + "url": "https://github.com/chalk/ansi-styles?sponsor=1" + } + }, "node_modules/source-map-js": { "version": "1.2.1", "resolved": "https://registry.npmjs.org/source-map-js/-/source-map-js-1.2.1.tgz", @@ -3321,6 +3668,62 @@ "dev": true, "license": "MIT" }, + "node_modules/string-argv": { + "version": "0.3.2", + "resolved": "https://registry.npmjs.org/string-argv/-/string-argv-0.3.2.tgz", + "integrity": "sha512-aqD2Q0144Z+/RqG52NeHEkZauTAUWJO8c6yTftGJKO3Tja5tUgIfmIl6kExvhtxSDP7fXB6DvzkfMpCd/F3G+Q==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=0.6.19" + } + }, + "node_modules/string-width": { + "version": "8.2.2", + "resolved": "https://registry.npmjs.org/string-width/-/string-width-8.2.2.tgz", + "integrity": "sha512-GaPUh5gfdrYzqeVNZvUfT23vYYxXzKYidUcnMtJg/3rxRV63EFZy3k6xfKlmfeJD0176lnUV/Usr3XcwSvFzpg==", + "dev": true, + "license": "MIT", + "dependencies": { + "get-east-asian-width": "^1.5.0", + "strip-ansi": "^7.1.2" + }, + "engines": { + "node": ">=20" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, + "node_modules/strip-ansi": { + "version": "7.2.0", + "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-7.2.0.tgz", + "integrity": "sha512-yDPMNjp4WyfYBkHnjIRLfca1i6KMyGCtsVgoKe/z1+6vukgaENdgGBZt+ZmKPc4gavvEZ5OgHfHdrazhgNyG7w==", + "dev": true, + "license": "MIT", + "dependencies": { + "ansi-regex": "^6.2.2" + }, + "engines": { + "node": ">=12" + }, + "funding": { + "url": "https://github.com/chalk/strip-ansi?sponsor=1" + } + }, + "node_modules/strip-ansi/node_modules/ansi-regex": { + "version": "6.2.2", + "resolved": "https://registry.npmjs.org/ansi-regex/-/ansi-regex-6.2.2.tgz", + "integrity": "sha512-Bq3SmSpyFHaWjPk8If9yc6svM8c56dB5BAtW4Qbw5jHTwwXXcTLoRMkpDJp6VL0XzlWaCHTXrkFURMYmD0sLqg==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=12" + }, + "funding": { + "url": "https://github.com/chalk/ansi-regex?sponsor=1" + } + }, "node_modules/strip-indent": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/strip-indent/-/strip-indent-3.0.0.tgz", @@ -3816,6 +4219,37 @@ "node": ">=0.10.0" } }, + "node_modules/wrap-ansi": { + "version": "10.0.0", + "resolved": "https://registry.npmjs.org/wrap-ansi/-/wrap-ansi-10.0.0.tgz", + "integrity": "sha512-SGcvg80f0wUy2/fXES19feHMz8E0JoXv2uNgHOu4Dgi2OrCy1lqwFYEJz1BLbDI0exjPMe/ZdzZ/YpGECBG/aQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "ansi-styles": "^6.2.3", + "string-width": "^8.2.0", + "strip-ansi": "^7.1.2" + }, + "engines": { + "node": ">=20" + }, + "funding": { + "url": "https://github.com/chalk/wrap-ansi?sponsor=1" + } + }, + "node_modules/wrap-ansi/node_modules/ansi-styles": { + "version": "6.2.3", + "resolved": "https://registry.npmjs.org/ansi-styles/-/ansi-styles-6.2.3.tgz", + "integrity": "sha512-4Dj6M28JB+oAH8kFkTLUo+a2jwOFkuqb3yucU0CANcRRUbxS0cP0nZYCGjcc3BNXwRIsUVmDGgzawme7zvJHvg==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=12" + }, + "funding": { + "url": "https://github.com/chalk/ansi-styles?sponsor=1" + } + }, "node_modules/xml-name-validator": { "version": "5.0.0", "resolved": "https://registry.npmjs.org/xml-name-validator/-/xml-name-validator-5.0.0.tgz", @@ -3840,6 +4274,22 @@ "dev": true, "license": "ISC" }, + "node_modules/yaml": { + "version": "2.9.0", + "resolved": "https://registry.npmjs.org/yaml/-/yaml-2.9.0.tgz", + "integrity": "sha512-2AvhNX3mb8zd6Zy7INTtSpl1F15HW6Wnqj0srWlkKLcpYl/gMIMJiyuGq2KeI2YFxUPjdlB+3Lc10seMLtL4cA==", + "license": "ISC", + "optional": true, + "bin": { + "yaml": "bin.mjs" + }, + "engines": { + "node": ">= 14.6" + }, + "funding": { + "url": "https://github.com/sponsors/eemeli" + } + }, "node_modules/yocto-queue": { "version": "0.1.0", "resolved": "https://registry.npmjs.org/yocto-queue/-/yocto-queue-0.1.0.tgz", diff --git a/web/package.json b/web/package.json index 2edbbf009..4d7e28050 100644 --- a/web/package.json +++ b/web/package.json @@ -10,7 +10,14 @@ "check:api": "npm run generate:api && git diff --exit-code -- src/api/generated/v1.d.ts", "lint": "eslint .", "test": "vitest", - "typecheck": "tsc -b --pretty false" + "typecheck": "tsc -b --pretty false", + "prepare": "cd .. && husky" + }, + "lint-staged": { + "src/**/*.{ts,tsx}": [ + "eslint --fix", + "bash -c 'npm run typecheck'" + ] }, "dependencies": { "@vitejs/plugin-react": "6.0.3", @@ -32,7 +39,9 @@ "eslint-plugin-react-hooks": "7.1.1", "eslint-plugin-react-refresh": "0.5.3", "globals": "17.7.0", + "husky": "^9.1.7", "jsdom": "29.1.1", + "lint-staged": "^17.0.8", "typescript-eslint": "8.62.1", "vitest": "4.1.9" } diff --git a/web/src/api/collections.ts b/web/src/api/collections.ts index f22d4b7ca..ca3b0ee71 100644 --- a/web/src/api/collections.ts +++ b/web/src/api/collections.ts @@ -1,4 +1,4 @@ -import { ApiError, request } from './client'; +import { ApiError, request, requestWithMeta, type ResponseWithMeta } from './client'; import type { components } from './generated/v1'; import type { LibraryBrowseItem, PagedLibraryBrowseItems } from './libraryBrowse'; @@ -57,12 +57,36 @@ export function getCollectionItems( return request(`/api/collections/${id}/items?${params.toString()}`); } +/** Load a page of collection items together with the collection's concurrency ETag (issue #253). */ +export function getCollectionItemsWithMeta( + id: number, + pageNum = 0, + pageSize = 100 +): Promise> { + const params = new URLSearchParams({ + pageNum: String(pageNum), + pageSize: String(pageSize) + }); + return requestWithMeta(`/api/collections/${id}/items?${params.toString()}`); +} + // Replaces a manual collection's custom order wholesale: the CustomIndex of each media item is // derived from its position in `mediaItemIds`, so a partial array silently drops the items left // out of it (#211). Callers must submit the full ordered id list. -export function updateCollectionCustomOrder(id: number, mediaItemIds: number[]): Promise { +// +// 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 updateCollectionCustomOrder( + id: number, + mediaItemIds: number[], + ifMatch?: string | null +): Promise> { const body: UpdateCollectionCustomOrderRequest = { mediaItemIds }; - return request(`/api/collections/${id}/custom-order`, { body, method: 'PUT' }); + return requestWithMeta(`/api/collections/${id}/custom-order`, { + body, + method: 'PUT', + headers: ifMatch ? { 'If-Match': ifMatch } : undefined + }); } /* ---------- smart collections ---------- */ 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/generated/v1.d.ts b/web/src/api/generated/v1.d.ts index d4ee00744..2a1ed580b 100644 --- a/web/src/api/generated/v1.d.ts +++ b/web/src/api/generated/v1.d.ts @@ -801,6 +801,7 @@ export interface components { "id": number; "name": null | string; "useCustomPlaybackOrder": boolean; + "version"?: number; "mediaItemId"?: number; "title"?: null | string; "subtitle"?: null | string; @@ -1154,6 +1155,7 @@ export interface components { "shuffleScheduleItems": boolean; "randomStartPoint": boolean; "fixedStartTimeBehavior": components["schemas"]["FixedStartTimeBehavior"]; + "version": number; }; "RemoteConnectionResponseModel": { "address": null | string; diff --git a/web/src/api/multiCollections.ts b/web/src/api/multiCollections.ts index 848ec03f9..9f6660304 100644 --- a/web/src/api/multiCollections.ts +++ b/web/src/api/multiCollections.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 MultiCollection = components['schemas']['MultiCollectionResponseModel']; @@ -38,12 +38,29 @@ export function getMultiCollection(id: number): Promise { return request(`/api/multi-collections/${id}`); } +/** Load a single multi-collection together with its concurrency ETag (issue #253). */ +export function getMultiCollectionWithMeta(id: number): Promise> { + return requestWithMeta(`/api/multi-collections/${id}`); +} + export function createMultiCollection(body: CreateMultiCollectionRequest): Promise { return request('/api/multi-collections', { body, method: 'POST' }); } -export function updateMultiCollection(id: number, body: UpdateMultiCollectionRequest): Promise { - return request(`/api/multi-collections/${id}`, { body, method: 'PUT' }); +/** + * Update a multi-collection. 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 updateMultiCollection( + id: number, + body: UpdateMultiCollectionRequest, + ifMatch?: string | null +): Promise> { + return requestWithMeta(`/api/multi-collections/${id}`, { + body, + method: 'PUT', + headers: ifMatch ? { 'If-Match': ifMatch } : undefined + }); } export function deleteMultiCollection(id: number): Promise { diff --git a/web/src/api/playlists.ts b/web/src/api/playlists.ts index bc6b4af49..c6535ab70 100644 --- a/web/src/api/playlists.ts +++ b/web/src/api/playlists.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 AddItemsToPlaylistRequest = components['schemas']['AddItemsToPlaylistRequest']; @@ -41,13 +41,28 @@ export function getPlaylistItems(id: number): Promise { return request(`/api/playlists/${id}/items`); } +/** Load playlist items together with the playlist's concurrency ETag (issue #253). */ +export function getPlaylistItemsWithMeta(id: number): Promise> { + return requestWithMeta(`/api/playlists/${id}/items`); +} + export function createPlaylist(body: CreatePlaylistRequest): Promise { return request('/api/playlists', { body, method: 'POST' }); } -// PUT = rename + replace the full item list; returns the persisted (re-indexed) items. -export function updatePlaylist(id: number, body: ReplacePlaylistRequest): Promise { - return request(`/api/playlists/${id}`, { body, method: 'PUT' }); +// PUT = rename + replace the full item list; returns the persisted (re-indexed) items. 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 updatePlaylist( + id: number, + body: ReplacePlaylistRequest, + ifMatch?: string | null +): Promise> { + return requestWithMeta(`/api/playlists/${id}`, { + body, + method: 'PUT', + headers: ifMatch ? { 'If-Match': ifMatch } : undefined + }); } export function deletePlaylist(id: number): Promise { diff --git a/web/src/api/playouts.ts b/web/src/api/playouts.ts index 529800061..5d5e08aab 100644 --- a/web/src/api/playouts.ts +++ b/web/src/api/playouts.ts @@ -1,5 +1,5 @@ import { useCallback, useEffect, useRef, useState } from 'react'; -import { ApiError, request } from './client'; +import { ApiError, request, requestWithMeta, type ResponseWithMeta } from './client'; import type { components } from './generated/v1'; export type PlayoutSummary = components['schemas']['PlayoutListItemResponseModel']; @@ -164,13 +164,26 @@ export function getAlternateSchedules(playoutId: number): Promise(`/api/playouts/${playoutId}/alternate-schedules`); } +/** Load alternate schedules together with the playout's concurrency ETag (issue #253). */ +export function getAlternateSchedulesWithMeta( + playoutId: number +): Promise> { + return requestWithMeta(`/api/playouts/${playoutId}/alternate-schedules`); +} + +/** + * Replace a playout's alternate schedules. 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 replaceAlternateSchedules( playoutId: number, - body: ReplacePlayoutAlternateSchedulesRequest -): Promise { - return request(`/api/playouts/${playoutId}/alternate-schedules`, { + body: ReplacePlayoutAlternateSchedulesRequest, + ifMatch?: string | null +): Promise> { + return requestWithMeta(`/api/playouts/${playoutId}/alternate-schedules`, { body, - method: 'PUT' + method: 'PUT', + headers: ifMatch ? { 'If-Match': ifMatch } : undefined }); } @@ -178,13 +191,24 @@ export function getPlayoutTemplates(playoutId: number): Promise(`/api/playouts/${playoutId}/templates`); } +/** Load playout templates together with the playout's concurrency ETag (issue #253). */ +export function getPlayoutTemplatesWithMeta(playoutId: number): Promise> { + return requestWithMeta(`/api/playouts/${playoutId}/templates`); +} + +/** + * Replace a playout's templates. 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 replacePlayoutTemplates( playoutId: number, - body: ReplacePlayoutTemplatesRequest -): Promise { - return request(`/api/playouts/${playoutId}/templates`, { + body: ReplacePlayoutTemplatesRequest, + ifMatch?: string | null +): Promise> { + return requestWithMeta(`/api/playouts/${playoutId}/templates`, { body, - method: 'PUT' + method: 'PUT', + headers: ifMatch ? { 'If-Match': ifMatch } : undefined }); } diff --git a/web/src/api/rerunCollections.ts b/web/src/api/rerunCollections.ts index c49da59e4..45d025497 100644 --- a/web/src/api/rerunCollections.ts +++ b/web/src/api/rerunCollections.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 RerunCollection = components['schemas']['RerunCollectionResponseModel']; @@ -36,12 +36,29 @@ export function getRerunCollection(id: number): Promise { return request(`/api/rerun-collections/${id}`); } +/** Load a single rerun collection together with its concurrency ETag (issue #253). */ +export function getRerunCollectionWithMeta(id: number): Promise> { + return requestWithMeta(`/api/rerun-collections/${id}`); +} + export function createRerunCollection(body: CreateRerunCollectionRequest): Promise { return request('/api/rerun-collections', { body, method: 'POST' }); } -export function updateRerunCollection(id: number, body: UpdateRerunCollectionRequest): Promise { - return request(`/api/rerun-collections/${id}`, { body, method: 'PUT' }); +/** + * Update a rerun collection. 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 updateRerunCollection( + id: number, + body: UpdateRerunCollectionRequest, + ifMatch?: string | null +): Promise> { + return requestWithMeta(`/api/rerun-collections/${id}`, { + body, + method: 'PUT', + headers: ifMatch ? { 'If-Match': ifMatch } : undefined + }); } export function deleteRerunCollection(id: number): Promise { diff --git a/web/src/api/schedules.ts b/web/src/api/schedules.ts index 4aac218a7..b87ba5b28 100644 --- a/web/src/api/schedules.ts +++ b/web/src/api/schedules.ts @@ -1,4 +1,4 @@ -import { ApiError, request } from './client'; +import { ApiError, request, requestWithMeta, type ResponseWithMeta } from './client'; import type { components } from './generated/v1'; // FillerPreset is exported by ./pickers — import (don't re-export) to avoid a duplicate `export *` // name in api/index.ts. LanguageCode/getLanguages moved to ./languages (also re-exported via @@ -46,17 +46,31 @@ export function getScheduleItems(scheduleId: number): Promise(`/api/schedules/${scheduleId}/items`); } +/** Load schedule items together with the schedule's concurrency ETag (issue #253). */ +export function getScheduleItemsWithMeta( + scheduleId: number +): Promise> { + return requestWithMeta(`/api/schedules/${scheduleId}/items`); +} + export function addScheduleItem(scheduleId: number, body: ScheduleItemRequest): Promise { return request(`/api/schedules/${scheduleId}/items`, { body, method: 'POST' }); } -// Destructive replace: the server deletes+recreates every item row (new ids) and triggers playout -// rebuilds. The editor batches all local draft edits into this single call. See docs/decisions.md. +// Positional in-place reconcile: the server reuses same-typed item rows (keeping fill-group state) and +// triggers playout rebuilds. The editor batches all local draft edits into this single call. 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). See docs/decisions.md. export function replaceScheduleItems( scheduleId: number, - body: ReplaceScheduleItemsRequest -): Promise { - return request(`/api/schedules/${scheduleId}/items`, { body, method: 'PUT' }); + body: ReplaceScheduleItemsRequest, + ifMatch?: string | null +): Promise> { + return requestWithMeta(`/api/schedules/${scheduleId}/items`, { + body, + method: 'PUT', + headers: ifMatch ? { 'If-Match': ifMatch } : undefined + }); } export function deleteScheduleItem(scheduleId: number, itemId: number): Promise { 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